After doing my reading assignment of Lintschool, I read some fantastic articles about Code review.
Code review is a process where team members start reading what you've done for a certain feature and start writing comments and mention some things, it's like a final check before the code is merged, wherein this check there are a couple of things to be checked (that rhymed).
- In code reviews, one usually looks at the style things are written, does it match the guidelines the team agreed to follow?
- is there something wrong with the logic?
- do the tests match the required business requirements or if there is a missing requirement that the tests didn't cover
- does the implemented design in code match the one that was handed by the designer?
this process is essential for any team as I said in the last post, it helps to build trust and knowledge of the codebase.
But there are some things that hinder this process...
Massive Pull Requests
Massive pull requests usually occur when you are working on a big feature alone.
This results in some issues for the reviewer, having to read through large files due to a large pull request is tedious, you just want to get to the end of it and get back to your work, and when the reviewers feel this, you will just give an over-the-surface review
while small and chunked PRs will give you an effective review of what you were missing, and leave too little room for bugs to hide.
But this takes us to the next question, how do we optimize our Code Review process?
Well, there are a couple of good practices when creating PRs
first things first, It is your responsibility to make it easy for your reviewers to review your code, they can't spend as much time as you did digging into the context of the problem your PR is trying to solve, therefore you need to present your PR in a friendly form, hence comes the Pull Request Template, you can find lots of PR templates here you can edit these templates to your use case and add it to your pull requests, doing so gives clear information about what this PR is trying to achieve and what goals it met, so the reviewer knows what the code at hand should output, it also gives you a clear mind of what happend in this PR and organize your train of thoughts of what is needed to tell reviewers I did that
2. Starting right is better than modifying midway
Splitting PRs is a hard process when it occurs midway when you find out that the reviewers won't review your changes because they are simply too big, don't get frustrated when that happens, you should make the effort in tidying it up and make it more reviewable
3. Let the reviewer follow your thought process from the commit messages
commit messages are super important in Code Reviews, they are the history of the PR, not just they allow the reviewer to follow in your footsteps, but if you happen to get your PR large, you can split the PR by cherrypicking the branch's commits and to do that easily you need to leave good commit messages to help you do so.
4. Mistakes to avoid
we all do mistakes and to avoid them, you must know them beforehand, here are some mistakes you can avoid in the future...
Too big features (i.e: long feedback loop) You're violating Agile, Agile is mostly about keeping your feedback loo short, you should do what you can to keep it short whenever, use pair programming, sit with the product designers and make sure you understand what they mean and want, make sure you write test cases before you start digging into code and show those test cases to your lead so that when you write the code you know they match the business requirements
You did too much work (i.e: Violating the YAGNI) you worked too much on enhancing the feature, or on cleaning the code, that's still a violation, but cleaning the code should be little by little like the boy scouts rule, leaving the campground cleaner than we found it
It turned out to be harder than estimated
New code wasn't covered by tests Tests have one important aspect, which is documenting the code written, sometimes the way to show the reviewers what the code is about is showing them the tests, if you miss it, they can spend a lot of time figuring out the consequences of every line of code you wrote in the PR, to avoid this mistake, you should introduce a proper test suite to your codebase
Those are some of the things you should follow when creating changes you want to merge to the codebase, but if you happen to make a massive pull request, you should definitely read this article about it
Splitting PRs is a good action that can help you minimize the headaches for your teammates a lot when they are reviewing, and it is your responsibility to make the reviewing process for your teammates easy and quick because they have other work to do than reviewing your PR.