Self Code Review
When I submit a change for code review, I take time to review it myself before tagging any of my teammates. Of course I look through the code in my editor before deciding I’m finished with the change, but that is only the first step. After I think the change is ready, I push my changes to Github and open a draft pull request. Using an interface designed for code review puts me in a critical mindset and helps me find issues I didn’t notice in the editor.
The first time I read through the changes, I take note of any obvious issues that aren’t related to the functionality of the code. This includes comments that shouldn’t be there, unused code, typos, and debug logging I didn’t intend to leave. I clean up those problems in one commit and push that. With basic cleanliness out of the way, the change looks like a real candidate for merging.
In the second reading, I focus on functionality. Did I miss anything expected from the ticket? Do I see any incorrect behavior or potential performance issues? Did I include tests for everything that should have them? I might only need to make minor changes to comments or logging or I may realize I missed something and need to go back and make bigger changes.
For simple changes I stop here, but for larger changes I review again as if I were looking at somebody else’s work. Pretending I didn’t just write this code isn’t always easy, but it is important to consider how the change looks from the outside. The focus here is on clarity and maintainability. I ask myself if the purpose of each function is clear, if naming conventions make sense, and if there are better ways to structure the code. Frequently I don’t find anything to change in this step, but still find it useful.
Only after completing these rounds of self-review do I request a review from my teammates. There are two major reasons I do this. The first is simply out of respect. My teammates don’t want to get a notification email when I push a commit to fix typos or realize the tests are failing because I forgot to run them before opening the PR. I don’t want to waste their time with spelling mistakes, unused code, or any simple issue I should have found on my own. I owe it to them to make sure the change is reasonable before I involve them in the process.
The second reason is I believe it improves the quality of the resulting code. If the change is already the best I can make it before I ask for help from my teammates, they’re less likely to get bogged down trying to understand the change or commenting on little things I could have noticed on my own. Without that overhead, there’s more opportunity to look for big-picture improvements and general code quality. It shortens the actual review process and helps ensure any comments they need to make are for substantive issues.
This practice certainly isn’t necessary, but I think my teammates and I benefit from it so I’m happy this is a habit I got myself into.