Code reviews are a great practise to find mistakes, potential bugs and other issues overlooked by the code author. Reviewing improves the overall code quality, but it is also a great way to share knowledge and information. Junior developers' commits should certainly be reviewed by more senior developers, but juniors should also be the reviewers. Here's why.
In every project there are things that need familiarization when a developer (junior or not) joins the team. These can include for instance code and project organization, testing, build, deployment and the general way of working in the project. Especially in large projects, understanding the existing architecture and design decisions as well as the rationale behind these decisions can be a major challenge for a new developer.
Both the technical aspects and the way of working can usually be found from the project documentation. Nevertheless, implementing new features or fixing bugs in an unfamiliar code base is still challenging. What would be a better way to learn than following the example of more experienced developers in the same project?
What would be a better way to learn than following the example of more experienced developers in the same project?
Having young developer go through commits of experienced developers by reviewing is a great learning experience for the reviewer. Not only does it give a great reference for design and architectural decisions as well as coding and style practices, but it also shows how to apply these in the actual project in which the developer is working on. For the reviewee, it does not hurt to have an extra set of eyes looking at the code even though the reviewer is not so experienced. In fact, someone who is not so familiar with the project most likely looks at it from a fresh perspective.
A lot of the benefits from this kind of review goes to the junior developer. However, there are also benefits for the project. Obviously the developer can familiarize quicker, but the reviews also give a wider view to the project than what would be directly necessary for the task at hand. Chances are that the now new developer will at some point maintain the project, so having a wider perspective from the get go is invaluable.
So far the discussion has been solely technical. Yet, the review practices can also have a social effect. When a new developer is asked to review commits of already established members, it shows that they are accepted as a member of the team and their input and opinions are valued. As always, successful review practices require healthy working environment. Everyone should be able to both give and receive feedback without taking the comments personally. To reinforce this atmosphere it is good to have everyone from juniors to seniors to be both reviewers and reviewees.
These same practices can also be applied across teams and across people with different competences. Why not have embedded developers review some GUI code or backend developers some front-end changes. Just because something is beyond someones' main competence area, does not mean they would be unable to spot mistakes or give feedback (of course some more subtle mistakes that relate to a specific library or language would still need reviewer with strong competence in that area).
So let's make the most of code reviews and have everyone from juniors to seniors take part.
Top comments (11)
This is so essential, I loath to think that there are teams out there where juniors are not reviewing the code of more senior members!
My very first programming job was working with a team that was in charge of a very large server product that involved everything from HTML+CSS and Ruby on Rails right down to video codecs and network protocols implemented in C/C++. When I was hired on, I had assumed it was primarily due to my experience with Ruby, so I was caught off guard a bit when one of our senior-most developers called me into his office the first week to review a commit he had just written for the work distribution algorithm that involved BSD socket APIs. I will admit, as the article here mentions, that I probably benefited more from the interaction, as I was about as useful in that code review as a Rubber Duck, but having that experience (and on the first week) cemented in my mind that we were one team and that, although I might have been hired for my Ruby skills, I was going to have to bear the same responsibility for our product as everyone else.
From that point on, I worked harder on that project than almost any other project I've worked on since.
Thanks for sharing your story! It coins exactly the point I was trying to make. It also shows a great professionalism from that senior developers part (probably one reason he/she was a senior in the first place) which, it seems, had a pretty profound effect.
I agree with you and thought the same thing. It's one of those intangible traits all senior developers should have. When I started as a developer, you tend to think senior means writing the most/best code and that's it. That should be a small part of it, absolutely. However, if you're not making those around you better, you're not being the best you could be.
It's the classic case of: a 10x developer doesn't produce 10x the code, they influence the 10 developers closest to them to produce 2x the code.
This is how we do things at my work - juniors can review senior dev's pull requests. It helps to break down walls and prevent authority from piling up.
We've just started doing this as well. Seems to get a bit competitive at times though.
I'm experiencing the same thing, but I think its worth it.
Love it. Great way to challenge traditional power structures and get people working together better.
This is absolutely necessary to do. You're not going to be magically bug-free just because you're more senior. Hell, hubris can breed bugs in your code just as easily as naiveté.
I never underestimate a new developer's ability to find bugs in my code. We all make mistakes. The more eyes on our programs, the better.
Great article and idea!