Welcome!

Blog Feed Post

Better Code Reviews Through Empathy

better code reviewshttps://www.pagerduty.com/wp-content/uploads/2017/04/bettercodereview-30... 300w, https://www.pagerduty.com/wp-content/uploads/2017/04/bettercodereview-76... 768w, https://www.pagerduty.com/wp-content/uploads/2017/04/bettercodereview-25... 250w, https://www.pagerduty.com/wp-content/uploads/2017/04/bettercodereview-18... 180w, https://www.pagerduty.com/wp-content/uploads/2017/04/bettercodereview.png 1518w" sizes="(max-width: 452px) 100vw, 452px" />Code reviews are an important part of the modern software lifecycle. Unfortunately, a lot of cycles are burned and morale is damaged because there are few guidelines given to reviewers (and reviewees) on constructive feedback and effective written communication. Below are some tips for a better code review process.

Determine why you do code reviews

Is it to find bugs? Is it to spread knowledge of the codebase? Is it to find architectural problems? What do code reviews offer your team? If you don’t know the answer to this question, you don’t know how well they’re working.

It’s probably best to have this explicit conversation with your team. It’s OK that teams use them for different things, but it is very important that both the author and reviewer know what the expected outcome of a review is.

Within our team, I expect that I’m sharing code mostly to spread knowledge of changes to codebases, and as a sanity check that I’m doing nothing incredibly stupid. Sometimes I will need expert guidance (for example, in front-end work, or in services that I rarely poke around in). Usually, I try to get this before tagging someone in a PR. When this happens, I think it is best to acknowledge explicitly that you’re expecting that person to be the expert, so that they can take a closer look and perhaps help you with a deploy.

Automate your style guide

If the answer to, “why you do code reviews” includes, “making sure the code follows a consistent style,” cut it out. It’s difficult to review code that is poorly formatted. And it’s really, really tough to review code for both correctness and style without making multiple passes. It’s also really demoralizing to new engineers on a project to look at their PR and see dozens of style comments. Great automated style tools exist that can automatically style your code and you should use these. On my particular team, we strive to use Scalariform for our Scala stuff and Rubocop for our Ruby projects. Under no circumstances in 2017, should engineers be manually fixing formatting issues.

As a fallback, in cases where automation has not been set up, it’s best for the reviewer to correct the formatting themselves instead of adding comments to the author (this is doubly true when you are reviewing code for your own teammate). Small things can be handled directly in the GitHub UI and we shouldn’t be shy about adding a few pointless commits that’ll be squashed down before merging anyway.

Offer suggestions, not orders

Don’t tell a committer how they’ve done a thing incorrectly in a code review remark. Offer a suggestion, and ask what they think of it. For example, take a look at the comments below:

Don’t repeat this here, extract a function.

This clearly feels very negative. The author may feel defensive and start a pointless argument, even if there are few benefits to doing so; or worse, the author may feel like a less valuable contributor, and follow the suggestion blindly. In either case, actual harm is done by comments like this.

The best thing you can do is to offer a suggestion, not as a gatekeeper, but as someone who is trying to learn about the changes to the code (even if you really are a gatekeeper).

What do you think about taking this with the above and extracting a function?

Why should you make your comments sound so tentative? Because you are reviewing your peer’s code. You have to invite them to politely explain what’s going on, or to notice that you’ve made a better suggestion. Having no manners in this regard eliminates the possibility of discussion for a significant segment of the population.

Let’s talk about my number one pet peeve in code reviews…

Why didn’t you…

Asking a “why didn’t you” question implies a great many things, none of which are constructive or positive. It assumes that the author of the code has previously considered your suggestion and explicitly chose a different one, which may not be the case. It implies that your alternative is obviously superior (especially in the special case of “why didn’t you just…“), and that you require that the author justify their solution against it. It implies that what may be intended as a mere suggestion is, in fact, the default.

Whenever I am reviewing code and my hubris needs to be checked, I try to offer a suggestion. “How do you feel about…” or “what do you think about…” are good drop-in replacements for this phrase.

Use overtly positive language

It is important to recognize that written communications have a profoundly negative bias. We tend to interpret neutral communications as negative, and positive communications as neutral, which is a source of a lot of consternation in code reviews. To combat this, strive for overt positivity.

Terseness can be interpreted as rude or inconsiderate. In particular, avoid adding a two or three word comment.

Use code reviews as a learning exercise

My genuine unsupported-by-data opinion is that code quality and correctness are only marginally improved by having a code review by a gatekeeper. It is rare that a reviewer shares the same level of insight as the person who authored the code (well, except in the aforementioned, “review by expert”). A better model is to have code reviews treated as a learning exercise for the reviewer, i.e. to understand the changes to the code and to grow in understanding of the code base. I think this leveling gives their questions and remarks appropriate weight, and allows for a healthier discussion about the larger codebase and how the changes interact with it.

Of course, you don’t have to do any of that to be nicer in your reviews.

When receiving a code review

For receiving a code review, you should follow the same suggestions in reverse. Recognize that terse comments don’t mean the reviewer thinks less of you. Recognize that “orders” are usually just poorly-phrased suggestions, and an invitation for you to think about the problem a (potentially different) way. Recognize that pretty much everything written comes off more negatively than intended, and give a lot of leeway to your teammates.

It’s easy to imagine that brutal code reviews are a ritual that leads to the best outcomes. In my experience, engineers are a lot more sensitive to criticism than they let on, and they are a lot more willing to be found wrong if they feel like collaborators rather than defendants. Take a few extra minutes to add some kindness to your reviews, and I think you’ll find your reviews are a lot more productive and leave you feeling better at the end of the day.

 

 


*This post was originally featured on Cory Chamblin’s personal blog. We thought it would be helpful to our blog readers, so we decided to share it here as well.

The post Better Code Reviews Through Empathy appeared first on PagerDuty.

Read the original blog entry...

More Stories By PagerDuty Blog

PagerDuty’s operations performance platform helps companies increase reliability. By connecting people, systems and data in a single view, PagerDuty delivers visibility and actionable intelligence across global operations for effective incident resolution management. PagerDuty has over 100 platform partners, and is trusted by Fortune 500 companies and startups alike, including Microsoft, National Instruments, Electronic Arts, Adobe, Rackspace, Etsy, Square and Github.

Latest Stories
Internet of @ThingsExpo, taking place October 31 - November 2, 2017, at the Santa Clara Convention Center in Santa Clara, CA, is co-located with 21st Cloud Expo and will feature technical sessions from a rock star conference faculty and the leading industry players in the world. The Internet of Things (IoT) is the most profound change in personal and enterprise IT since the creation of the Worldwide Web more than 20 years ago. All major researchers estimate there will be tens of billions devic...
"The Striim platform is a full end-to-end streaming integration and analytics platform that is middleware that covers a lot of different use cases," explained Steve Wilkes, Founder and CTO at Striim, in this SYS-CON.tv interview at 20th Cloud Expo, held June 6-8, 2017, at the Javits Center in New York City, NY.
"With Digital Experience Monitoring what used to be a simple visit to a web page has exploded into app on phones, data from social media feeds, competitive benchmarking - these are all components that are only available because of some type of digital asset," explained Leo Vasiliou, Director of Web Performance Engineering at Catchpoint Systems, in this SYS-CON.tv interview at DevOps Summit at 20th Cloud Expo, held June 6-8, 2017, at the Javits Center in New York City, NY.
21st International Cloud Expo, taking place October 31 - November 2, 2017, at the Santa Clara Convention Center in Santa Clara, CA, will feature technical sessions from a rock star conference faculty and the leading industry players in the world. Cloud computing is now being embraced by a majority of enterprises of all sizes. Yesterday's debate about public vs. private has transformed into the reality of hybrid cloud: a recent survey shows that 74% of enterprises have a hybrid cloud strategy. Me...
SYS-CON Events announced today that Datera, that offers a radically new data management architecture, has been named "Exhibitor" of SYS-CON's 21st International Cloud Expo ®, which will take place on Oct 31 - Nov 2, 2017, at the Santa Clara Convention Center in Santa Clara, CA. Datera is transforming the traditional datacenter model through modern cloud simplicity. The technology industry is at another major inflection point. The rise of mobile, the Internet of Things, data storage and Big...
SYS-CON Events announced today that DXWorldExpo has been named “Global Sponsor” of SYS-CON's 21st International Cloud Expo, which will take place on Oct 31 – Nov 2, 2017, at the Santa Clara Convention Center in Santa Clara, CA. Digital Transformation is the key issue driving the global enterprise IT business. Digital Transformation is most prominent among Global 2000 enterprises and government institutions.
Kubernetes is an open source system for automating deployment, scaling, and management of containerized applications. Kubernetes was originally built by Google, leveraging years of experience with managing container workloads, and is now a Cloud Native Compute Foundation (CNCF) project. Kubernetes has been widely adopted by the community, supported on all major public and private cloud providers, and is gaining rapid adoption in enterprises. However, Kubernetes may seem intimidating and complex ...
"Outscale was founded in 2010, is based in France, is a strategic partner to Dassault Systémes and has done quite a bit of work with divisions of Dassault," explained Jackie Funk, Digital Marketing exec at Outscale, in this SYS-CON.tv interview at 20th Cloud Expo, held June 6-8, 2017, at the Javits Center in New York City, NY.
"We focus on SAP workloads because they are among the most powerful but somewhat challenging workloads out there to take into public cloud," explained Swen Conrad, CEO of Ocean9, Inc., in this SYS-CON.tv interview at 20th Cloud Expo, held June 6-8, 2017, at the Javits Center in New York City, NY.
"I think DevOps is now a rambunctious teenager – it’s starting to get a mind of its own, wanting to get its own things but it still needs some adult supervision," explained Thomas Hooker, VP of marketing at CollabNet, in this SYS-CON.tv interview at DevOps Summit at 20th Cloud Expo, held June 6-8, 2017, at the Javits Center in New York City, NY.
"We are still a relatively small software house and we are focusing on certain industries like FinTech, med tech, energy and utilities. We help our customers with their digital transformation," noted Piotr Stawinski, Founder and CEO of EARP Integration, in this SYS-CON.tv interview at 20th Cloud Expo, held June 6-8, 2017, at the Javits Center in New York City, NY.
"We've been engaging with a lot of customers including Panasonic, we've been involved with Cisco and now we're working with the U.S. government - the Department of Homeland Security," explained Peter Jung, Chief Product Officer at Pulzze Systems, in this SYS-CON.tv interview at @ThingsExpo, held June 6-8, 2017, at the Javits Center in New York City, NY.
"We're here to tell the world about our cloud-scale infrastructure that we have at Juniper combined with the world-class security that we put into the cloud," explained Lisa Guess, VP of Systems Engineering at Juniper Networks, in this SYS-CON.tv interview at 20th Cloud Expo, held June 6-8, 2017, at the Javits Center in New York City, NY.
Your homes and cars can be automated and self-serviced. Why can't your storage? From simply asking questions to analyze and troubleshoot your infrastructure, to provisioning storage with snapshots, recovery and replication, your wildest sci-fi dream has come true. In his session at @DevOpsSummit at 20th Cloud Expo, Dan Florea, Director of Product Management at Tintri, provided a ChatOps demo where you can talk to your storage and manage it from anywhere, through Slack and similar services with...
As enterprise cloud becomes the norm, businesses and government programs must address compounded regulatory compliance related to data privacy and information protection. The most recent, Controlled Unclassified Information and the EU’s GDPR have board level implications and companies still struggle with demonstrating due diligence. Developers and DevOps leaders, as part of the pre-planning process and the associated supply chain, could benefit from updating their code libraries and design by in...