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
DX World EXPO, LLC, a Lighthouse Point, Florida-based startup trade show producer and the creator of "DXWorldEXPO® - Digital Transformation Conference & Expo" has announced its executive management team. The team is headed by Levent Selamoglu, who has been named CEO. "Now is the time for a truly global DX event, to bring together the leading minds from the technology world in a conversation about Digital Transformation," he said in making the announcement.
"Space Monkey by Vivent Smart Home is a product that is a distributed cloud-based edge storage network. Vivent Smart Home, our parent company, is a smart home provider that places a lot of hard drives across homes in North America," explained JT Olds, Director of Engineering, and Brandon Crowfeather, Product Manager, at Vivint Smart Home, in this SYS-CON.tv interview at @ThingsExpo, held Oct 31 – Nov 2, 2017, at the Santa Clara Convention Center in Santa Clara, CA.
SYS-CON Events announced today that Conference Guru has been named “Media Sponsor” of the 22nd International Cloud Expo, which will take place on June 5-7, 2018, at the Javits Center in New York, NY. A valuable conference experience generates new contacts, sales leads, potential strategic partners and potential investors; helps gather competitive intelligence and even provides inspiration for new products and services. Conference Guru works with conference organizers to pass great deals to gre...
DevOps is under attack because developers don’t want to mess with infrastructure. They will happily own their code into production, but want to use platforms instead of raw automation. That’s changing the landscape that we understand as DevOps with both architecture concepts (CloudNative) and process redefinition (SRE). Rob Hirschfeld’s recent work in Kubernetes operations has led to the conclusion that containers and related platforms have changed the way we should be thinking about DevOps and...
The Internet of Things will challenge the status quo of how IT and development organizations operate. Or will it? Certainly the fog layer of IoT requires special insights about data ontology, security and transactional integrity. But the developmental challenges are the same: People, Process and Platform. In his session at @ThingsExpo, Craig Sproule, CEO of Metavine, demonstrated how to move beyond today's coding paradigm and shared the must-have mindsets for removing complexity from the develop...
In his Opening Keynote at 21st Cloud Expo, John Considine, General Manager of IBM Cloud Infrastructure, led attendees through the exciting evolution of the cloud. He looked at this major disruption from the perspective of technology, business models, and what this means for enterprises of all sizes. John Considine is General Manager of Cloud Infrastructure Services at IBM. In that role he is responsible for leading IBM’s public cloud infrastructure including strategy, development, and offering m...
The next XaaS is CICDaaS. Why? Because CICD saves developers a huge amount of time. CD is an especially great option for projects that require multiple and frequent contributions to be integrated. But… securing CICD best practices is an emerging, essential, yet little understood practice for DevOps teams and their Cloud Service Providers. The only way to get CICD to work in a highly secure environment takes collaboration, patience and persistence. Building CICD in the cloud requires rigorous ar...
Companies are harnessing data in ways we once associated with science fiction. Analysts have access to a plethora of visualization and reporting tools, but considering the vast amount of data businesses collect and limitations of CPUs, end users are forced to design their structures and systems with limitations. Until now. As the cloud toolkit to analyze data has evolved, GPUs have stepped in to massively parallel SQL, visualization and machine learning.
"Evatronix provides design services to companies that need to integrate the IoT technology in their products but they don't necessarily have the expertise, knowledge and design team to do so," explained Adam Morawiec, VP of Business Development at Evatronix, in this SYS-CON.tv interview at @ThingsExpo, held Oct 31 – Nov 2, 2017, at the Santa Clara Convention Center in Santa Clara, CA.
To get the most out of their data, successful companies are not focusing on queries and data lakes, they are actively integrating analytics into their operations with a data-first application development approach. Real-time adjustments to improve revenues, reduce costs, or mitigate risk rely on applications that minimize latency on a variety of data sources. In his session at @BigDataExpo, Jack Norris, Senior Vice President, Data and Applications at MapR Technologies, reviewed best practices to ...
Widespread fragmentation is stalling the growth of the IIoT and making it difficult for partners to work together. The number of software platforms, apps, hardware and connectivity standards is creating paralysis among businesses that are afraid of being locked into a solution. EdgeX Foundry is unifying the community around a common IoT edge framework and an ecosystem of interoperable components.
"ZeroStack is a startup in Silicon Valley. We're solving a very interesting problem around bringing public cloud convenience with private cloud control for enterprises and mid-size companies," explained Kamesh Pemmaraju, VP of Product Management at ZeroStack, in this SYS-CON.tv interview at 21st Cloud Expo, held Oct 31 – Nov 2, 2017, at the Santa Clara Convention Center in Santa Clara, CA.
Large industrial manufacturing organizations are adopting the agile principles of cloud software companies. The industrial manufacturing development process has not scaled over time. Now that design CAD teams are geographically distributed, centralizing their work is key. With large multi-gigabyte projects, outdated tools have stifled industrial team agility, time-to-market milestones, and impacted P&L stakeholders.
"Akvelon is a software development company and we also provide consultancy services to folks who are looking to scale or accelerate their engineering roadmaps," explained Jeremiah Mothersell, Marketing Manager at Akvelon, in this SYS-CON.tv interview at 21st Cloud Expo, held Oct 31 – Nov 2, 2017, at the Santa Clara Convention Center in Santa Clara, CA.
Enterprises are adopting Kubernetes to accelerate the development and the delivery of cloud-native applications. However, sharing a Kubernetes cluster between members of the same team can be challenging. And, sharing clusters across multiple teams is even harder. Kubernetes offers several constructs to help implement segmentation and isolation. However, these primitives can be complex to understand and apply. As a result, it’s becoming common for enterprises to end up with several clusters. Thi...