How to do an effective code review?
When you come to your new job, you always look at your first review commits as they have been written in other languages. You don’t know the company’s frameworks and business domain yet; you aren’t familiar with your colleagues’ habits.
Your first several code reviews might not be effective.
The question is how to come to an effective code review finally. Where is the point you’d like to go to? I’ll try to answer it.
Your code review is effective when you understand each line of code, and you understand if tests are working just by looking at them and their implementation.
Are you agree with it? If not, share your thoughts!
The remark here is that tests should cover every line of production code.
Well, how do I achieve this state? Especially if, at first, you have the slightest idea what your colleague has done.
1.First, just read all lines and check for duplicates / human errors. I’m not joking. You can save an incident in your company with a proper review even without fully understanding your colleague’s code. You can find a typo and save hours of debugging and tackling an issue.
I personally had a case when after improper code review, my colleague deployed code with a type which caused a dreadful production incident. So, I revisit my view on the code review.
2. Check if the code line is covered by the test. That can save several attempts to deploy incorrectly working applications. Even and especially if the culture of your company TDD and BDD, your colleague can forget to test a new method. It’s annoying to do so; let’s be honest by being the most hateful reviewer asking colleagues to cover every line by tests. At least you can sleep safely, as you personally did a proper code review.
I personally had a case when I made a deployment for incorrect code, and I could easily avoid it by just providing a proper test. My reviewer noticed it and requested changes in the next PR. I realised that covering my three code lines with twenty lines of testing code gave me another hint to fix and improve those three lines. And I understood the code base of my colleagues much better. Thanks to the reviewer who put me in the right direction.
3.For every test, ask yourself why this test is working. If you can’t answer by yourself, ask your colleague exactly the same question: “why is this test working?”. Again, you may save several hours of finding an issue in the production in case tests are done incorrectly and the code doesn’t work as expected by passing incorrect tests.
I personally had a case when I asked my much-more-senior colleague why the test was working. And it appeared that he didn’t run this test locally; instead, he just published the code for the type of test he would have seen only after merging if I didn’t point him in the wrong direction.
To sum up, I hope these notes will give you a guide if you don’t know where to start and are completely confused by your colleagues’ PRs in a new company. Then, as soon as you feel the ground under your legs and you understand what your colleagues are doing. If you feel that a proper review is boring and or too brain-consuming, and you struggle to keep reading your colleagues’ code every day, remember: you can save hours of fixing improperly developed implementations!