DEV Community

Code review checklist

Udayakumar Rayala on July 04, 2018

http://blog.smartbear.com/wp-content/uploads/2015/09/Creating-Your-Code-Review-Checklist.jpg I have posted in my last blog about how we do code ...
Read full post
Collapse
 
eljayadobe profile image
Eljay-Adobe •

In my experience, code reviews provide a gloss level review of the change or enhancement.

Also a giant impact is if the change is small, the code review will be of higher value.

If the change is large, the code review will be of little value.

If the change is gigantic, the code review will be of infinitesimal value.

Unless the code reviewer is well-versed and up-to-date on that area of code, it's easy to code review the picayune nits and complete miss the pink elephants in the code.

Whereas, anecdotally, in the few times that I participated in pair programming, which entails continuous code reviews, the code quality was far higher. The continuous code review provided significantly more value.

Here's a couple interesting essays about code reviews, from Microsoft:

Collapse
 
rafalpienkowski profile image
Rafal Pienkowski •

Very nice list.

I think that list such as this are always handy. Fortunately I know that most of devs are lazy (and this is not a drawback) and I could add such rules but they won't use them if they don't have to. To avoid such a situation
when nobody follows such a list I'm adding it to my pull request policy. I always setup such as policies:

  • build based on feature/bugfix branch has to be succeeded
  • all tests have to be "green"
  • there shouldn't be any new code smells or code duplications after static code analysis
  • author cannot accept own changes
  • at least two developers are required to accept changes

I think that we should automate as much as possible and focus on things which can not be autometed like:

  • checking functional and nonfunctional requirements
  • checking if the code could be refactored
  • wear the tester hat (I like this term)

Last but not the least, always encourage all team members to actively participate in code reviews regarding their experience. Sometimes junior dev could find something interesting and there is also great place to knowledge and experience sharing.

To sum up, I think that this list is a great starting point for a pull request policy in a project.

Cheers.

Collapse
 
uday_rayala profile image
Udayakumar Rayala •

The idea of the post was to bring common understanding to the team about what to look for in pull request reviews. Because everyone has their own understanding of what to check for in the code reviews, it is good to write down things to share knowledge and also act as an agreement. Such a list should be owned by the team and enhanced further.

I agree with automating as much as possible and such automated checks should be part of the build pipeline.