One Way of Doing Code Reviews

Introduction

When it comes to code reviews, it all depends on your company and your team's mode of work, or the process that moves an idea into a software product.

With this article, I am presenting one way of doing code reviews, it is basically the way I do it, and I believe lots of developers have done it this way as well. I will first pinpoint the basic steps and then go through them in more detail.

I am assuming here that code reviews are made against code that is submitted with tests, indifferent to the way of development (test first, test last, ...).

In summary, the steps are as follows:

1. Check if tests exist.

2. Make sure all tests pass.

3. Validate that the tests meet the acceptance criteria.

4. Validate that the tests are properly written.

5. Check the code for improvement suggestions. (Cleaner code, better structure, performance bottlenecks, un-used parts, etc..).

6. Provide feedback to your colleagues using available tools.

I will now go through each of them in more details.

Check If Tests Exist

This is the most basic step and I believe it should be a requirement to never move your code into a ready for review state unless you have tests. The simple reasoning behind that is that your colleague cannot make sure that your code does what it promises to do, and without this minimum guarantee there is no point in reviewing the code.

Make Sure All Tests Pass

Now you can run the tests and make sure that nothing fails. This prevents colleagues from submitting buggy code, buggy tests or code with regression issues.

In case you have a CI/CD pipeline, the pipeline should include tests and this pipeline (if properly set) should cover other types of tests. 

Validate That the Tests Meet the Acceptance Criteria

The third step is basically a simple rundown of the acceptance criteria and the available tests. Of course, this depends on the process (company, team), so if you use BDD tests it is much easier to check that all cases are covered. 

Of course, this step assumes that you read very carefully the acceptance criteria and you have at least an idea of what kind of scenarios to cover.

Communication is key in all phases of development, so you will find yourself asking questions to your team colleagues, the business people, the product owners/managers to make sure that what is written in the acceptance criteria is the same as what the tests are validating.

The unit tests are covered in a later step.

Validate That the Tests Are Properly Written

Imagine all the previous steps are fine but then you notice that the written tests do not validate anything, or that the setup is wrong, or that some validations are missing. Sometimes the code setup (mocks or DB data setup or others) is very cumbersome that you even need to debug to check that the test data is valid first before checking the assertions against them later on.

In this phase, you will find yourself digging deeper into the code and assessing it, not just the way the tests are setup but also a bit of the implementation. Which eventually leads you to toggle between this step and the last one.

Check the Code for Improvement Suggestions

While checking the tests (especially BDD tests) you will start going deeper into the code. You will take notes about improvements in implementation algorithms, code structure, clean code, un-used parts, possible performance bottlenecks, and improvements.

You will look into the unit tests and validate them against the acceptance criteria (as sometimes they share some logic).

Provide Feedback to Your Colleagues Using Available Tools

This is the final step of the suggested process, now that you have assessed everything and you are ready to provide feedback. A few things can happen in this phase:

• You are happy with everything; you just approved it and move the task/sub-task to done.

• You have some minor comments. Depending on the available tools you either have to discuss your suggestions directly with your colleagues or add some comments on your code collaboration board.

• You are suggesting big changes. In this case, communication is key, you discuss your changes with your colleagues, make sure you are correct about your assumptions, and the change is in fact needed, and then you can go into implementation according to the agreed-upon process. This could be pair programming, you do the change, your colleague does the change; it all depends on the agreed process.

Conclusion

This is just one way of doing code reviews and it assumes a lot of givens. I have made this post to get feedback, discuss different perspectives, and maybe find improvements and try them.