For number of years now I have seen many types of reviews happen and how people in different companies do there code reviews and id like to share some of the good and bad things that really can make the difference to create good quality code.
Note : this is can be a highly emotive topic in some places and don't want to cause upset but there are some things that objectively bad and some things that are great.
I'm primely a C# engineer and with it be a compiled language before we get to code review we have a number of checks in place that can stop a lot of the headaches. (This will be why there are somethings that will more relevant to some than others 🙂) so I'm not going to talk language specific things just general good and bad that I've seen work or not work.
Leaving commented code (Bad)
This causes extra complexity when looking at code in the future. The whole reason for source control is to be able to remover or add code and be able to check the history if necessary
Understand the Context (Good)
Should always check the ticket/story or if needs be talk to the dev that did it to get an understanding of what the code is for. This will always lead to a better code review as you will be able to give more relevant feedback.
Nit-picking (Bad)
Being picky for pick sake will always be bad if you holding up the completion of a code review cause of something that dose change the functionality of the code then that can cause high tension between developers or have knock on effects for release.
just to be clear I'm not talking about things like being consistent with in a project or documented Code Styles they should be followed.
Check for Readability & Maintainability (Good)
Having readable code is important consistent with in a project and documented Code Styles really help with this so that when other devs come to look at a project it makes it quick and simpler for them to pick and work on.
Inconsistent Standards (Bad)
As i have mentioned a few times now its important to be consistent keeping that running through a project really helps to reduce time to make changes, add features or event debug problems so its very important that when you see inconsistent code from how the rest of the project is done that its highlighted and challenged.
Provide Constructive Feedback (Good)
Give specific, actionable suggestions for improvement. Avoid vague comments like "this could be better.".
Make an attempt to suggest how it could be better (without solving the problem) look to offer links to resources or places in the project that where they can take an example from.
The best examples I've seen are where devs have offered a suggestion to the problem with links to docs or "Have you considered x =+ NewLog;" using the suggestion tools that GitHub and many others have