In Messente, we don't enforce super strict code review rules, but rather rely on common sense and just a few friendly guidelines.
Be kind and helpful
The most fundamental part of code reviews is to be kind, helpful and avoid toxic behaviour. Sandya Sankarram has written an excellent article called "Unlearning toxic behaviours in a code review culture" - this reading is a must!
I am not going to duplicate everything she says, but this is just a super short recap of the helpful behaviours to live by:
- Helpful Behavior #1: use questions or recommendations to drive dialogue
- Helpful Behavior #2: collaborate, don’t back-seat drive
- Helpful Behavior #3: respond to every comment
- Helpful Behavior #4: know when to take a discussion offline
- Helpful Behavior #5: use opportunities to teach, and don’t show off
- Helpful Behavior #6: don’t show surprise
- Helpful Behavior #7: automate what can be
- Helpful Behavior #8: refuse to normalize toxic behaviour
- Helpful Behavior #9: managers — hire carefully, listen to your team, and enforce
- Helpful Behavior #10: set the standard as your team is small and growing
- Helpful Behavior #11: understand you might be part of the problem
Creating pull requests and asking for a code review
Code reviews are tough, so a lot of effort has to be put into requesting one as well. The person doing code review is taking time from his own development, so you have to respect that and reward the effort by being as helpful as possible.
1) Provide basic background info
Whenever you ask for a code review, make sure to describe what this pull request (PR) does in general and include a short description of what and why things were changed.
If testing the code requires some kind of setup or configuration, then include the instructions in the pull request or link to a wiki page that describes the process. Documentation is a crucial part of your code anyway, right? :)
2) Make it as easy as possible
Create as small PR as possible, so that it would be easier to review. If this is impossible due to the nature of the project itself, then you could easily create easy to follow commit log which describes what you are doing in every step, so that reviewer can track the development process as well. Not just the huge diff at the end.
One part of "common sense" that is worth also mentioning - don't request review for buggy code. You are responsible for your own code, so make sure it works and you have done basic testing.
Reviewing pull request
This is the basic list of guidelines to follow when doing a code review:
- If the changed feature isn't too small then try to run it in the staging environment and test it out. Don't count on automated tests to catch everything.
- Don't go crazy with testing if the change is small. Setting up a testing environment to test the font style change might be a waste of time.
- You should review PRs assigned to you at least once per day. It's good to create a habit so other devs know when to expect the review (for example checking it every morning/afternoon).
- If you're not familiar with the project or don't have time to do the review then ask the person who opened PR to assign it to somebody else.
- Code formatting should not be a part of the code review process - we rely on code formatter for this.