paint-brush
Don’t be the bad cop in pull request reviews — Let software do that jobby@mikenikles
3,205 reads
3,205 reads

Don’t be the bad cop in pull request reviews — Let software do that job

by Mike NiklesJune 9th, 2018
Read on Terminal Reader
Read this story w/o Javascript
tldt arrow

Too Long; Didn't Read

When was the last time you reviewed a pull request and reminded the author to add a link to your bug tracking system? How about adding a missing label? Maybe you found an open PR that had no reviewer / assignee set.

Companies Mentioned

Mention Thumbnail
Mention Thumbnail
featured image - Don’t be the bad cop in pull request reviews — Let software do that job
Mike Nikles HackerNoon profile picture

And try to celebrate achievements!

Credit: Internet

When was the last time you reviewed a pull request and reminded the author to add a link to your bug tracking system? How about adding a missing label? Maybe you found an open PR that had no reviewer / assignee set.

I’ve certainly been there and for a long time didn’t think much when I wrote comments like that. At times, I didn’t even leave a comment and simply fixed it. It wasn’t until a few years ago when I started to officially lead engineering teams that I started to pay attention to my own, but also my team’s productivity and overall happiness at work.

Fictional Jimmy constantly tells me what to do

Who’s fictional Jimmy? It could be anyone, often though senior engineers or someone who’s been with the company for a while (I have been and probably still am fictional Jimmy once in a while, sorry team!). It’s that person who adds comments to PRs such as “Please mention the bug ticket # in the PR title” or if you’re in a less friendly environment, things like “Missing link to Jira!” or “Labels!”.

Fictional Jimmy means well, he really does. Unfortunately, new team members or people with less work experience may not understand that. See, someone who’s worked in the software industry for many years understands the importance of consistency, well defined processes and to a certain degree the need to be picky. It’s an understanding we slowly build over time.

To authors who receive feedback like that, it may not be a big deal — at first… Over time though, I truly believe humans start to feel negativity in such comments. This leads to completely unnecessary tension nobody speaks about.

Well then, what can we do about that?

Danger to the rescue

Danger runs during your CI process, and gives teams the chance to automate common code review chores.

This provides another logical step in your build, through this Danger can help lint your rote tasks in daily code review.

You can use Danger to codify your teams norms. Leaving humans to think about harder problems.

This happens by Danger leaving messages inside your PRs based on rules that you create with JavaScript or TypeScript.

Over time, as rules are adhered to, the message is amended to reflect the current state of the code review.

That’s the official description taken from Danger’s website. I recently took a moment to install and configure Danger. Their Getting Started guide contains the information you need. Alternatively, add danger as a dependency to your project and run yarn danger init or the equivalent command for NPM. It’s an interactive command line tool that guides you through the setup process. It takes less than half an hour.

Before I go on, let me clarify that Danger (just like fictional Jimmy) adds comments to pull requests to remind people of things they forgot to do, improvements they can make to the pull request, etc. I believe the psychological difference in that message coming from a human vs. a piece of software makes a big difference though. Also, as you will see as you continue to read, we can use Danger in a way that it makes encouraging and motivating comments when people do things well — or even automate some common tasks!

Our initial dangerfile.js

Changes to the continuous integration

In most cases, this is as simple as adding yarn danger ci to your pipeline. The Danger website has examples for many CI systems.

Results in pull requests

As part of the setup process, you will create a Danger bot user. When yarn danger ci runs, that user posts the results as a comment to the pull request.

Danger results posted as a PR comment

Let’s say I fix the last issue and add the ticket URL to the PR body. The next time yarn danger ci runs, it updates the existing comment.

Notice the edited dropdown menu? It’s a new feature Github introduced and it goes very well with Danger’s way of updating its own comment.

Danger’s comment edit history

Celebrate achievements

As you can see at the top of our Dangerfile.js, I ask team members to review http://danger.systems/js/usage/culture.html. It’s a great summary of what becomes increasingly important: Team happiness.

Software engineers face challenge after challenge, dead end after dead end. Failed tests, bugs, performance & security issues and product requirement changes are only a few situations to deal with. Now here’s another tool that posts a pull request comment and tells me what to do. Why not use Danger to celebrate achievements?

Let the team know when they do well. It’s a small gesture that goes a long way! Some thoughts:

  • More code is deleted than added.
  • A developer opened a PR to improve documentation.
  • Someone modified that piece of code nobody else wants to touch.
  • What about sharing a new, random motivational quote similarly to what Slack does when you load it?

But wait, Danger can do more!

So far, we looked at Danger’s capabilities to analyze a PR and let authors know what to do or celebrate their achievements.

While reading the API reference documentation, I noticed the following comment:

// An authenticated API so you can extend danger’s behavior by using the GitHub v3 API.

It’s available at danger.github.api and provides a vast list of functionality, which is documented in detail here.

In our case, we have a monorepo that contains NPM packages as well as services. To visualize what pull requests modify, we have labels for each package and service. Authors of a PR are supposed to add a label for each package and service they modify.

I knew from the beginning that I can’t ask my team to do that manually. I wouldn’t want to do it if I was writing as much code as my team does, so why would I ask them to do it?

With Danger, this can now be automated! It’s always in sync and nobody has to do anything manually. The code is too specific to our repo structure to share in its entirety, but it roughly looks like this:





// Get a list of all files in this PRconst allTouchedFiles = danger.git.created_files.concat(danger.git.modified_files,danger.git.deleted_files)


// Determine the unique packages / services that changedconst uniquePackagesAndServices = [/* Your way of figuring that out */]




// Add labelsdanger.github.api.issues.addLabels({labels: uniquePackagesAndServices})

Conclusion

With Danger, we once again automated small pieces of repetitive work that used to take us time or caused frustration at various degrees. We even have a tool to celebrate micro successes as part of individual pull requests.

What’s most promising to me is danger.github.api as this opens up the door to potentially very nice automated processes.

Do you use Danger? I’d love to see your Dangerfile and learn what you do with it. Let me know in the comments.

👏 ❤️