A guide for Code Reviews

CR Submission Guidelines

Keep your CR focussed. If possible send CR per feature. If the feature is big then divide feature into multiple CRs focussed on different aspects. For example, “user history feature” may require database access and user interface. So, you might send a CR covering two aspects. Lastly, send divided feature request ground up e.g. send database access before user interface.

Make sure that your code is self-explanatory and has adequate amount of code documentation and comments.

Test your code! Write your unit tests along with your code and include them in CR submission because adding test later often leads to bad code. Attach code coverage in testing if possible. There are plenty of coverage tools.

Give some context about your CR e.g. provide a description. If it’s a big change, arrange a design review meeting on white board. Make sure everybody understands and agrees on what you’re proposing.

Make use of issue tracking tool. Include related issue in your CR title, body and etc.

Post a new CR for unrelated changes. Don’t include them in the same CR. If it’s something you need to get it done urgently, create a new branch and post new changes from this branch.

Follow coding conventions and formatting for the language. You can have your team conventions and formatting but you need to be consistent with it.

Code Reviewing Guidelines

Spend some time in understanding what sender tries to accomplish. Read CR description, linked issue or any related design document before starting the code review.

Make your comments constructive. Make sure you give enough context about your concern or question so that sender won’t have any problem in understanding what’s the concern or question.

Prefer asking questions over stating issues. For instance, use “What was the reason behind not using standard library method?” instead of “You didn’t make use of the standard library method.”

Remember to praise! If you see code snippet function, design choice that’s good, indicate you like his/her approach.

Prefer design meetings instead of huge number of CR comments and change requests.

Remember there are multiple ways of coding a feature/function. So, don’t insist on your preferences. Let people do what they want when it comes to preference.

For both reviewer and sender