26 Aug 2020

People usually think that all a software engineer does all day is code but that isn’t the whole story. There is a very long list of lesser known but important tasks that take our time every day and it’s important to talk about them and highlight the contribution they bring to our work.


Prioritize Code Reviews
 

In my opinion, code reviews are important for many reasons, one of which is catching mistakes before they make it to Production. In some of my experiences, I feel that we don’t always give this valuable process the attention that it deserves.

No matter the seniority of a software engineer, code reviews are a responsibility for all contributors to the code base. Not because you will regularly find a potential bug while reviewing lines, but because it will help you to be more involved in the application, the workflows, and the business rules. There is also the added benefit of acquiring knowledge simply by being exposed to code written by other people.

Sometimes, we avoid doing code reviews because we are doing our own work and don't want to get distracted. We think, "I'll take a look later" and that can often turn into never. To avoid this trap, we need to put ourselves in the position of the author of the code. Consider that their work is finished and is waiting for your review. Feedback is needed in order to continue to the next ticket or user story, but that path is delayed when she doesn’t receive any insight on the delivered code.

It's important to not become a blocker for our teammates and to consider pull requests a priority. Take into account that, as a reviewer, if we send a change list to the author which will require time to solve, the sooner you finish your review the sooner the author can start making the requested changes.

As a team member, keep the goals of the sprint in mind and actively participate to achieve them. Prioritize the stories that need to be done first according to the planning and try to identify the ones that are dependents. This is going to help the project advance quickly and efficiently.

  

Battle the "LGTM" culture - Go further

 

It’s important to consider the positive impact a thorough code review brings to the project overall. The work from each individual is going to impact our work and the well-being of the whole application.

Look at syntax/code guidelines:  

As a reviewer, it is very important that we identify syntax and style issues in the code review. In my experience, most of the feedback on the code will focus on syntax. Also, check that the author is respecting the code guidelines you’ve set as a team. Maintaining this standard is a good way to keep bugs out of the code. A helpful way to do this is to add a linter or static analysis tools to the codebase so it’s easier to focus attention to the implementation of solutions, refactors and logic of the code.

Smart and open feedback  

Try to not to repeat comments that another reviewer has already pointed out. We can join the discussion suggesting a possible solution but remember, we are trying to help, not only point out mistakes. If you find a pattern of mistakes, compile them into a single comment instead of flagging each one individually. Be clear and constructive in your explanation so that the author easily understands and can identify the main problem without pointing out exactly all the places the mistake occurs. Most importantly, as an author, be open if any issues are pointed out because catching bugs before release is in the best interest of the project. Have a solution? Provide examples but be cool.

Remember, everyone thinks differently, and a problem can be solved in many ways - all of them ‘the right way.' If we reviewers know a more optimal way to solve a problem, be supportive and provide an example. This can be a sensitive situation, so take the same course I described when finding a pattern of mistakes and offer constructive critique. This requires effective communication through constructive feedback so that your suggestions are considered as areas of improvement and not misinterpreted by the author as aggressive or disrespectful.

As you take the role of the code reviewer more seriously, you’ll see that consistent contribution to the process and suggestions for improvement are essential to the process. We can do it better than just, “Looks good to me!” and it’s important to the team and the project altogether.

LGTM is easy, but it’s a bad habit and not the best way to support the quality of written code. Often when we work with agile methodologies, there is a belief that doing it faster is doing it better, but we must remember that sacrificing the quality of things to advance to the next will hurt us in the long run. Code reviews turn into complicated labor when we see it as a distractor and obligation when it's actually a valuable part of our job.

If you learn to handle the delicate situation of providing feedback during code reviews well, you will create stronger communication with the team and improved teamwork, overall.

Pull Requests - Scope  

 

As a reviewer, limit your feedback to the lines changed. Don’t ask the author to solve something that is out of the scope of the User Story. This is only going to increase the size of the pull request and confuse other reviewers.

As the author, respect the changes that you need to do for the story. Sometimes while we are coding, we find a piece of code that probably needs a refactor, and we think “well, I’m already here... what if...” Avoid this temptation of Piggyback Changes! They are a distraction that will cost time and money to the project creating tech debt and causing problems down the line.

Get the approvals! 

 

I've worked in different teams and with people with different personalities and approaches to their work, and a consistent problem that I encounter is the unwillingness to perform code reviews.

Those experiences caused me to stop and question the reasons why a pull request might not be reviewed. Personally, I find it discouraging when I review a pull request that actually does not provide any description, contains hundreds of files, and has no context.

Here are some suggestions that can help:

Make Friendly Pull Requests by Providing Context 

 

Clarify your work: 

1. Create a title encompassing in a few words the work you did. This is the first thing a reviewer will see. Example: Title: Update Users Data Model.

2. Use the description box and provide a short but concise description of your work from a technical viewpoint. This is the opportunity to explain yourself without the abstraction that we sometimes find in the description of the tickets.

3. To ensure #1 and #2 are always included, work with your team and create your own Pull Request Template. Github has a way to create templates for pull requests as a way to standardize the contributions especially for open source projects. This is a solid feature of the project documentation.

Here an example of template:

  • Title: According to the ticket number, you can specify which type of issue it is at the beginning (feature, bug, hotfix)
  • Description: Brief description of the ticket and what the code trying to accomplish. Include detailed information on the changes including, list the things done using the task list, using a numbered list or bulleted list. Try to include the Jira ticket link, if it exists.

Use the preview to see how it looks:

Don't open huge pull requests  

Another driver that pushes people to avoid reviews is seeing a huge pull request. It’s exhausting to check hundreds of files. In these instances, the definition of the scope might be too broad, or we are trying to solve too many issues at one time. A pull request with a lot of changes that contain different scopes is a bad practice and should be avoided. Take time to define the scope and the pull request should respect the story’s scope. Even if the scope is huge, break the pull request into smaller ones, focusing on small scopes and working with small tasks.

Additionally, commit your changes as soon as something is done or focus your commits per significant action. This is a best practice to follow in case you need to quickly roll back a specific batch of changes. Committing changes early also enables us to review the next set of changes after the first round of reviews.

Assign reviewers 

A good technique in my experience is assigning two team members at a time to be reviewers. Two pairs of eyes are better than one and often people find different things and while providing their observations. Organize this by setting your GitHub preferences to be able to only merge when a designated number of approvals exists.  

Keep things fresh by assigning reviewers randomly and then rotate their assignments to make sure everyone participates equitably. Choosing assignments based on skills is another way to refine the review process. For example, if your pull request is related to database changes, choose someone who has ample experience in that area.

Be open to change  

"Effective communication is the foundation of any working relationship"

 

Receiving feedback is an opportunity to improve our code skills. It allows us to see problems from a different point of view and to realize other solutions and approaches. Appreciate the feedback and try not to take it personally. Be open to change and talk with your reviewers about the comments you’ve received. And, if you feel that the suggestions from the reviews could be improved, say so. Everyone can improve their soft skills and candid, respectful communication will drive improvements.

Lack of response? Insist and remind 

Use opportunities like stand-ups or resend messages to your team channel to remind them that you are waiting for approval. Let them know you made the corresponding changes requested and you need someone to look at them.

Through my career, I have learned a lot through collaboration with team members. It’s not enough just to learn, but to apply what you have learned to support constant improvement. I’ve recently applied the guidance in the article to my current team and we’ve been seen a lot of good changes as a result. We’ll continue to adapt and grow together and respectful and constructive code reviews and pull requests are a big part of ongoing improvement.

Effective communication is the foundation of any working relationship and the coordination between code author and reviewer should be carefully managed. Do this by reinforcing ways that a team can improve code reviews and implement those that works best. This may take some trial and error, so stay present with your team, participate in discussions, and suggest ideas.

For a better world, with better code reviews....

 

 

COMMENTS