Some thoughts about code reviews

I want to tell you about the most important process we have in our team: Peer Code Review.

Code review is not just a process, it's a culture thing. As developers we should be willing to do it. Even if it's not to a peer, you should be reviewing Open Source Project's code, you will learn a lot.

I will write more in depth about how we should be doing code reviews in a future post, but now I'm going to talk about it's purpose, about the why:

  • It raises code quality, enforcing readibility, efficiency (through engineering) and maintanability.
  • It minimizes bugs. More people looking at the same code will minimize bugs pushed to production.
  • It encourages a feedback culture. All code should be reviewed, no matter what, no matter who wrote it. This helps us to grow and to create a constructive feedback culture.
  • It disperses knowledge through the whole team, raising the bus factor. At least two people understand the code.
I found out that code reviews are the most effective technique for team building. It empowers developers to do their best on each iteration and push the team as a whole to the next level.

Let's get back to the purpose of this post, what rules I learned from code reviews:

This is not a rule, but a state of mind:

Don't be an ass.

Having your code reviewed.

  1. Don't take it personally. This is about the code, not you.
  2. Embrace change.
  3. Be proud of your code.
  4. Push small. Small amount of code is easier to review than big full blown modules. Help the reviewer.
  5. Give context and why and what. Tell people why you decided to do something and, more important, explain the context of that decission. The documenation is very important for this. Document your code, explain the purposes of the modules / classes so it's easy for everybody to understand.
  6. Get context from the reviewer, understand the why and the what.
  7. Write good commit messages. Commit messages matter. Part of our job as engineers is to take the time to write good commit messages. It's not for us, it's for the future of the project. Take time and do it the right way. 

If you are the reviewer.

  1. Be respectful and have patience. (go and check the first rule). 
  2. Give recommendations (think of the goal).
  3. Ask questions, don't make demands.
  4. You can make it different... but... there are many ways to do the same thing.
  5. This is not a "Why" session. If you need to ask why, ask for context first. 
  6. Don't make people wait for you. Code reviews are very important, schedule time for code reviews.
  7. Take your time. Block at least one hour of your time to do it. This is important for the team, so take your time and do it the right way. 
  8. Approve the Pull Request! (or not). 

  • http://blog.codinghorror.com/the-ten-commandments-of-egoless-programming/
  • http://chris.beams.io/posts/git-commit/

No hay comentarios.:

Publicar un comentario