Code review process

Code Review and Continuous Integration

Every code change – even one liners – have to be reviewed! Switch to a new branch away from master. e.g.

git checkout -b feature/map_database_serialization

Work on your changes, try to keep them concise.

• Do not commit code that does not build.
• < 500 lines per change-list.
• Commit liberally.

Test and verify your changes.

• Use gTest (see here), focus on testing also less common cases.
• Think about wrong inputs to your methods and try to catch them.

Code review process

Detailed process: overview

Brief description:

As an author of a change:

• Commit your changes and push them to github: git push -u feature/map_database_serialization
• Issue a pull-request on github: creating-a-pull-request
• Ask your co-students or supervisor to review the code.
• Incorporate the comments and ask the reviewer to have another look after you incorporated the changes.
• After the reviewer has approved the changes, don't add additional code.
• Make sure your latest change is successfully tested by the build-server.
• Merge the branch to master by hitting the merge button.

As a reviewer of someone's pull-request:

• Look for memory-leaks, algorithmic errors, inefficiencies, indexing errors to name a few.
• Request changes for bugs, inefficient code etc.
• Approve the changes. (a common comment is "this lgtm" (this looks good to me)).
• The pull-request proposer now merges the code, when she/he is ready. (the author not the reviewer decides when to everything is ready for the merge).