The majority of developers do code reviews – some of them are at the point that most of their work is about doing code reviews. There are very few reasons when not having code review may be an acceptable idea, those can be:
- Proof of Concept code that will be used just to present technology or approach (it will be thrown away or rewritten, … right?)
- Design Docs and Proposals, the mixture of documentation, design diagrams, and the actual code that will be discussed with a larger audience later
- Your Pet projects, it is yours, you do what you want (anyway, asking colleague from the review can improve things a lot)
I’m going to split the list of the tips to Good Submitter and Good Reviewer, but there is general rule that is common for both sides.
0. Have a Style Guide
Teams usually have rules that they apply during code review, like:
“We open curly brackets in the next line, not in the same line.”
or
We do not nest more than two levels of “if” statements”
This kind of rule should be created, agreed upon Team, maintained, and easily accessible. Accessibility matters because the main purpose of this document is to walk it through every time you will be doing a review or submitting code. Having such a style guide prevents PR rejections caused by personal taste and introduces routine to this event.
There are many style guides already baked (C++, Python, Java, Shell, JS, HTML, …), that can be taken right away or modified to the needs -> Google Styleguides.
Great Submitter
1. Do Pre-Review
Looking at the style guide document, pre-read the changes that you are about to submit. You will most probably save yourself from “typo”, “unused var”, “method too long”, “not meaningful name”, and many more comments. You should validate your code against the style you agreed within the team. Sometimes, you can decide to change the design after reading your own code or you find some missing corner cases – it always helps!
2. Keep changes small
If you ever reviewed 50+ files in one pull request you what it is about. What is more, it doesn’t matter if we like big reviews or not, there is a study that shows that more than 500 Lines of Code (LOC) significantly reduce defect discovery. Of course, not always it will be possible, but then the time taken for the review should be increased to not go faster than 500 LOC/h.
3. Use Linter
Our IDE’s have it built-in or support it as a plugin, to show the code that does not meet our rules. There are many things that can be highlighted, some of them:
- code styling
- check for unreachable code
- check for debugger statement
- check for unused imports, vars
- check for unreachable code
- syntax checks
What is more – git can be configured to enforce lint checks for us and disable commit if checks are failed. You can do it using the pre-commit hook.
4. Detach yourself from your code
It may be frustrating when you were crafting your code, pre-readed it, and now somebody dragging it through the mud but you need to remember that all opinions and comments are about the code – not about you!
Not taking things personally is a simple way to feel better, but we can learn a lot when taking a “hard” code review as an opportunity.
5. Accept feedback
It doesn’t matter if you are Senior Dev, Architect, or just starting your adventure with coding – you should be prepared, and even more, awaiting feedback about your pull request. There are some things you can expect you to see as your feedback:
- Variable Names
- Documentation update
- Changelog/Release notes
- Optimizations
It is important to respond to the comments (even/first of all if you don’t agree with them) in a kind way. Arguing about minor things when more important things are there in the system may not be the best idea – however, it may depend on defect tolerance in the project.
6. Provide context
Having only code in code review may be a bit tricky to understand all of the processes and assumptions taken during development.
The most important context is the answer to the question: “Why I wrote this code?”, it can be linked GitHub issue, Jira Task, or a few sentences of explanation.
This rule can be applied also to the lower level – class, function, or some pieces of complicated code that cannot be simplified.
What next?
I hope the list above would be useful for you, as you can see – reviewing code is not just about code, it is also about people taking the best from it.
Being the “Great Submitter” is just one side of the coin – another one, being the “Great Reviewer” maybe even more complex (link to another post soon will be here!).