Code Review is a widely adopted practice in the software development industry. It represents the process of reviewing someone’s code before it gets merged into the existing code base. Whenever a developer wants to perform any sort of change, he or she must create a Pull Request (PR). The PR represents all of the changes made against the current state of the code base. Other developers in the team will then comment on the work done and accept/refuse the change. This is either done in person or via version control software. Code Review is a cornerstone of software development, whose pros severely outweigh the cons. In this article, we will discuss the advantages and disadvantages of Code Review and look at ways you can improve your Code Reviews. Why Code Review is Great Asset to a Team Collective Code Ownership First and foremost, Code Review is great at destroying silos inside the team. If you are not employing pair/mob programming regularly within your team chances are that your developers are working alone (which is absolutely fine). Whilst working alone it can be difficult to have in mind a complete overview of what’s being built because you’re down in the trenches making stuff work. It is also hard to escape your own mental biases that can lead you towards a particular design of a solution. Without Code Review, the code just needs to work and it is good to go. However, this increases the chances of making you, the author of the code, the sole person responsible for its future maintenance since you are the one who wrote it and can probably change it faster than anyone else. This leads to situations like “ ” which can hinder your team’s ability to maintain the code base and develop new features. Please ask Mark, he’s the only one who makes changes to the database schema With Code Review in place, every new addition to the code base is reviewed by someone who is taking a fresh look at your code. He or she will critique the decisions the author made and make sure the code follows the team’s guidelines and practices, effectively making sure that there is only one entity which owns the code, the team. It is important that people rather than . Code Review is not only about finding mistakes but also calling out good software engineering. critique criticize Collective Code Ownership means that everyone in the team has some degree of familiarity with distinct parts of the code base which makes the team cross-functional, facilitates interactions between members and speeds up development time. Fosters Collaboration Code Review forces people to communicate and collaborate. If your team is taking the reviews seriously then people are discussing ideas, patterns, ways to approach problems, the list goes on… It might not look like much but reviewing other people’s code is also a way to build trust and empathy in your team. It creates moments where the team needs to work together at the cost of halting the progress of development. It is also a way of easily integrating new people to your team’s way of doing things. A new person will, by default, receive feedback on his work, making it easier for them to adhere to the team’s standards. Improves Critical Thinking Reviewing people’s code is also a way of improving critical thinking. Most of the time we are either producing or maintaining code. Having moments where you need to critique other people’s code puts you in a position where instead of thinking you will think . This shift in mindset trains your critical thinking muscle. “what do I need to do to make this work?” “how can I improve this piece of code?” Prevents Defects This is one of the broadly accepted reasons that skyrocketed the popularity of Code Review. Code Review lets your team catch defects before they reach production. These are not your typical bugs, those should be caught by automated tools, but rather bugs and problems related to the code’s business logic, which are usually a bigger deal. missing semicolon In my experience, I have seen a lot of comments such as . These types of defects will not be caught by automated tools, you need humans looking at the code. “this will not work because service X requires Y and here we are sending Z” Improves Code Quality/Standards The quality of the code will increase as soon as you start doing Code Review. People will start reading code before it goes live so they can immediately point out its lack of readability or its flaws to comply with the team’s standards. As time goes on your team will also be on the lookout for what standards they can change and improve to make the process easier. If your team finds out they are spending a lot of time arguing over stylistic preferences they might decide to introduce a tool that deals with formatting. They can also integrate the version control software with a CI server that runs the build on each PR to see if everything is good. It’s usually a win-win situation. on Markus Spiske Unsplash What Should You Look Out For? Blocking Development This is one of the most common hurdles and main arguments against Code Review - . “It takes too much time, time that I can spend developing more stuff” You are trading off taking a little longer to develop new features (because those need to be reviewed) for higher quality code and code that adheres to the team’s standards and can be easily understood. That means more maintainable code in my book, and since most of the time we spent working is actually spent reading and maintaining code, it is a good trade-off. However, PRs can take too long to be merged to the code base if people don’t agree on an approach. This can halt development especially if the PR has code that other work depends on. Make sure you and your team are pragmatic in this scenario. It is better to ship something even though it may not be super clean than to not ship something at all. Keeping Things Short People who review code most of the time will understand and agree that effective PRs are short and sweet. This facilitates the job of the reviewer because he usually doesn’t have the full context of the code he’s reviewing, meaning that a smaller PR is way easier to reason about than a big PR. Big PRs are usually reviewed with less care because of the mental strain it imposes on reviewers. If people don’t review PRs carefully then you’re just throwing away the benefits of Code Review. Do everyone a favor and keep your PRs short. Nitpicking and Improper Behavior Code Review can be an outlet for nitpicking and improper behavior. I have seen situations where people argue too much over stylistic preferences, are condescending towards their teammates, make aggressive comments and hold PRs hostage effectively blocking the work of someone. There is no silver bullet for all of this. Stylistic discussions can be solved by using automated code formatting tools but people will still find other stuff to argue about if that’s what they enjoy doing. The other scenarios mentioned are harder to tackle because they are masked as “regular work”. Your team needs to be aware of the improper behavior and take action towards fixing it. It might be solved with a simple conversation or it might require outside intervention. on Nelson Pigossi Jr Unsplash Improve your Code Reviews Pros and cons aside, there are still some topics around Code Review that deserve some attention, more so if you are already doing Code Review. 1) Having Multiple Reviewers I have worked in teams where only one approval per PR was required and in teams where multiple approvals were required. I even argued, in the team where only one approval was necessary, that we should start having more than one person approve the PR. One of the reasons is that different people with different levels of experience will naturally look for different things when reviewing code. Junior people generally look at PRs in a less critical fashion, because they are probably still getting used to the code/technology/practices. However, they will pay attention to details such as typos and may ask more senior people on the team to leave comments when something is not easily perceptible. More experienced developers will have a more complete picture of the system and may find defects without even testing the code simply because they have the knowledge. I think one should always require the approval of one of the more experienced developers in the team. If you only have juniors reviewing and accepting PRs then you might introduce some defects simply because of the lack of knowledge of the system. Nonetheless, if you only have senior people reviewing PRs then you are missing out on the chance of the juniors to broaden their knowledge of the code base, which is one of the reasons for having multiple reviewers. Having multiple people reviewing the code also increases collective code ownership, and we have already seen why that’s good, but be aware of scenarios where people have preferred reviewers. I have been in a situation where two of the devs in the team were reviewing each other’s PRs, effectively shutting out other people in the team from the feature they were developing, even though the whole team is responsible for supporting it. Besides this, even if people are not super comfortable reviewing parts of code they have not seen before, simply giving it a look may help in future cases. There have been many times where instead of creating a new function from scratch I reused something because I remembered seeing that the team had done it previously somewhere. 2) Asking for Feedback Sometimes people are not 100% sure they can make a change in the code or they don’t know which pattern better suits the situation. In these types of scenarios, the programmer might wait for someone who has the knowledge to help out because he is afraid of submitting a PR and getting negative feedback. One thing that works really well is in this scenario is simply doing something which solves the task at hand and submitting a PR. Then, instead of waiting for people to leave comments, write comments in the places where you’ve written code you’re not sure it’s correct/best approach. Not only will you practice solving stuff by yourself but you will also ask for feedback in an asynchronous way, meaning that you avoided being idle while waiting for someone to help and you saved someone else’s time. Your colleagues will also appreciate your auto-critiques. 3) Reviewing Your Own Code I have created this habit of reviewing my own code before I create a new PR which I think is very beneficial not only one a personal level but also for the team. Firstly, I think reviewing your own code is proper code etiquette. People will invest their time reviewing your work so you should make sure that what you present for review is of quality to avoid wasting people’s time. Avoid silly mistakes such as dummy testing code or unnecessary logging. Secondly, it’s a good exercise to practice self-awareness and auto-critique. We are usually very critical of the code of others but when we are developing something we usually don’t hold ourselves in the same standard, mostly because when we are developing we are trying to reach a goal, not making sure the code is perfect. By reviewing your own code you will figure out your biases towards Code Review (because you will tend not to be as critical of yourself than others) and become a better reviewer. 4) Being Positive Following up to the last point, I think it’s too easy to only point out the negative aspects of code in Code Reviews. Besides this, Code Reviews are usually performed in an asynchronous fashion, meaning people are communicating via text, where they can be easily misinterpreted. Try to be positive in your Code Reviews, give thumbs up to good comments, leave a comment saying if the person did a good job and say something when someone wrote neat code or refactored an ugly function. 5) Don’t Rush It Even though Code Review can possibly block development, sometimes the other spectrum happens and people review code too fast. I’ve heard the phrase way too often. Even when crunching for a release or simply because other work is dependent on a specific PR, you shouldn’t rush the review, because that’s how bugs and inconsistencies slip by. “Can you please approve my PR?” Try asking instead. It removes the pressure from the reviewer to accept but still incites some urgency because you’re directly asking someone to take a look at it rather than being notified through other means that a PR exists. “Can you please review my PR?” In Summary, Code Review is an awesome and cheap way of improving the quality of your work and the quality of your team. Approach this process in a serious and positive way and I’m sure you will quickly reap the benefits. If you enjoyed this post please consider clapping for it and following me on Medium and Twitter . All feedback is appreciated!
