As developers, we perform a lot of code reviews (or at least we should be). They are a great way to not only ensure that the code we intend to deploy to production won't have a negative impact on the system, but also a time to learn and better ourselves.
Unfortunately, I have noticed a trend, especially at work. We tend spend the majority of our cycles verifying whether or not the code conforms to standards. In other words, our first instinct is to scan the code for incorrectly placed braces, improperly named variables, etc. Rather than review the actual quality of the code.
There's a lot of great resources available out there on this subject, so I don't want to regurgitate what's already been published, but I did want to highlight a couple points that this article speaks to.
What a code review is:
- An opportunity to highlight what is being done well. Reaffirm that the developer is taking the correct approach to solving the problem.
- An opportunity to highlight what can be done better. Offer different solutions and approaches and use it as a chance to improve one's skill set.
What a code review is not:
- A witch hunt, a time to point out every little fault. Code reviews are a great opportunity to learn. Developers should be eager to have their code reviewed, not shy away from it for fear of belittlement (even if unintended).
How to conduct a code review:
First and foremost, understand the subject matter. Really familiarize yourself with the problem that the reviewee is trying to solve. Go to the developer's office and immerse yourself in the problem. You can't offer solution suggestions if you don't even know what the problem is.
Focus on how the problem was solved. Not how it's formatted. Think big picture. Is the change the right approach to solve the problem in the grand scheme of things?
Code reviews that just point out every little standard violation aren't beneficial to anyone. Use code reviews as an opportunity to knowledge share, and encourage learning. Not as a platform to demonstrate how well you can enforce formatting guidelines.
I would also encourage you to seek out reviews from other developers, especially if you're even the slightest bit unsure if your solution makes sense. Just because you have the deploy permission, doesn't mean you're stuck in a silo. Go forth and get a second opinion.
If you're looking for additional information, here's a blog series that I completely agree with: