In my new team, we are working on several guidelines, rules and process improvements. Why do we think they are so important? If things are well documented, it’s easy for a newcomer to start giving value. This reduces the chances of errors for everyone. This removes a lot of possibilities for arguments. And we all know that no one can win an argument, so we should avoid them at all costs.

For a more detailed look at the importance of guidelines, please refer to this article. I’ll watch it again soon, by the way.

This time, I’m going to focus on code reviews and related guidelines.

purpose of code review
Reviewing pull requests is an important and sensitive task. In my opinion, it is at least as important as writing the code. Furthermore, reviewing someone else’s code is not only a technical task, but it is also a human task. This gives most of its delicacy.

So let me start with the most important rule that you should always keep in mind whenever you start reviewing a pull request or whenever you get a review:

No comment should be personal. No comments should be made about the author or the reviewer. A review should always be about the code!

The purpose of code review is to improve code, detect bugs before it is merged and distributed, and to improve maintainability of a given code base.

Items to be checked in a code review
Reviewing code is hard, and it’s a very broad task. According to my bosses, I am considered a good code reviewer. But still, I think my effectiveness could be greatly improved. I think the following checklist can be a great help in most cases.

Now, obviously, some of those checklists and/or tasks will be language-specific. However, reviews are helped by similar concepts that exist in many code languages.

These lists are mostly to give you some ideas, as they are far from complete. Feel free to use them, update them, personalize them, or just let them inspire you to come up with something completely new.

I think a reviewer should not use all of them, but maybe only a few. But if you have separate checklists, it’s easier to share tasks.

Not all checklists are used for all code reviews. If the pull request is really a small bugfix, then just improving one condition one by one would not require checking the design of the whole domain.

complete process checklist

It focuses on some of the basic features of a pull request. Make sure the new commit doesn’t break compilation or testing. Your Continuous Integration Pipeline should take care of this, but if it doesn’t – don’t forget about it.

safety checklist

Your application may or may not be security-critical. As soon as it is hacked once or it fails due to some messy input, it will become one. This checklist should pretty much be language dependent (I’m giving you one for C++).

Best Practices Testing Checklist
I hope we all agree that testing is part of a developer’s job. If we discuss testing, it will be about different ways of doing it, not whether we should do it or not.

The bad news is that the same path isn’t suitable for everyone. Still, I would recommend you to follow the cycle of Test Driven Development. The good news is that, on a project, there is at least a general understanding of what needs to be done.

If there isn’t one, step up and advocate for testing, collect articles and studies, and convince the team. You will be highly respected.

code readability checklist

We – the developers – are all authors. If we do a flawless job, our code will read like prose. I’m not saying that you should always reach this goal for the entire codebase, but you should aim for it.

Leave a Reply

Your email address will not be published. Required fields are marked *