Automating Code Review Chores Using Danger.js

Spend more time reviewing the critical parts of your pull requests.

Image by author

We, software engineers, spend a good chunk of time doing code reviews. And we should because reviewing code is an important activity that allows us to:

• Detect potential bugs before we release our code.
• Check if our code is doing what it’s supposed to do.
• Ensure our code is maintainable.
• Maintain consistent coding standards, e.g., code quality, unit tests, etc.

However, some parts of the code review can feel tedious if not automated. We can classify them as “code review chores.” Some examples are:

• Check if the author has updated a file as part of the pull request. e.g.CHANGELOG.mdandpackage.jsonfiles.
• Check if the author has added tests to the newly created functions.
• Check if the Pull Request (PR) is too big.
• Check for typos.

We don’t want to be that developer who keeps reminding everyone to keep the Pull Request (PR) size small or to keep thepackage.jsonfile updated. Code review chores become apparent when we repeat the same PR feedback.

We should spend more time reviewing the actual problem that the PR is trying to solve. So we can minimize the time spent on code review chores.

Automating the chores will improve our code’s quality and our team’s efficiency. The less we have to worry about chores, the more time we can spend reviewing the critical parts of the PR, like business logic and potential bugs.

Danger JS

Luckily, there is a free tool that automates code review chores calledDanger. It comes in many versions, likeDanger for RubyandDanger for JavaScript (Danger JS). Both versions do the same thing. It’s a matter of choosing your preferred programming language to write your danger rules. I’m going to use Danger JS in this post.

You can read the “What is Danger?” section in my previous blog post for a short intro to Danger.

I will share a few examples of common chores we can automate using Danger JS in the following sections.

Danger JS flow chart by the author.

Update Specific Files in the PR

For example: Ensure that we update theCHANGELOG.mdandpackage.jsonfiles in the PR. If you have a shared JavaScript library, keeping track of its changelog and version is a good practice.

Here’s what you can do in yourDangerfunction.

• Check if your PR includes changes to your changelog file andpackage.jsonfile.
• If there are no changes, leave a comment in the PR saying that files need to be updated.

Here’s some sample code fromdanger.systems/js:

// Add a CHANGELOG entry for app changes
const hasChangelog = danger.git.modified_files.includes("changelog.md")
const isTrivial = (danger.github.pr.body + danger.github.pr.title).includes("#trivial")

if (!hasChangelog && !isTrivial) { 
  warn("Please add a changelog entry for your changes.")
}

There is also a Danger JS extension for keeping track of the changelog file:danger-plugin-keepachangelog. You can use the extension directly, so there’s no need to write the function from scratch.

Unintended print statements

We use print statements to debug or test our changes locally. Unfortunately, we will sometimes commit these unintentionally to our branch.

It’s a good practice to exclude unnecessary print statements in your main branch. Redundant print statements add to the noise when running your app or service, making your codebase harder to debug.

Most of the time, they get spotted by the author or reviewer. However, inevitably, some of them can still go through. We should automate this check using Danger JS.

Here’s the pseudocode for reference:

// Get the modified files using DangerJS
modifiedFiles = getModifiedFiles()
for (modifiedFile in modifiedFiles) { 
  // Get the added diff for each file using DangerJS 
  diffAddedForFile = getDiffAdded(modifiedFile) 
  // Check if the diff's string includes a print statement e.g. fmt.print in Go or console.log in JS. 
  if (diffAddedForFile.includes('fmt.print')) { 
    warn('fmt.print detected') 
  } 
}

Note that other alternative solutions can be used if you use JavaScript, so you don’t have to write your Danger rule from scratch.

• Danger JS extension:danger-plugin-noconsole.
• ESLInt’s no-console.

Check if the PR is too big

We want our PRs to be as small as possible. Large PRs increase cognitive load. Smaller PRs are more manageable, making them easier to review. Sometimes, we must remind the PR author to break down their PR into smaller chunks.

We can write a Danger JS rule that will count the number of lines changed in a PR. If the number exceeds a specified “Big PR threshold,” leave a comment to break down the PR into smaller PRs.

Lines of change = number of deletions + number of additions

It’s up to your team to agree on an acceptable Big PR threshold. Start with a threshold that your team arbitrarily feels is good enough to start with. In my experience, I usually start with a 500 to 600 value. Then fine-tune it over time.

Here’s a sample code from:https://danger.systems/js/:

var bigPRThreshold = 600;
if (danger.github.pr.additions + danger.github.pr.deletions > bigPRThreshold) { 
  warn(':exclamation: Big PR (' + ++errorCount + ')'); 
  markdown('> (' + errorCount + ') : Pull Request size seems relatively large. If Pull Request contains multiple changes, split each into separate PR will helps faster, easier review.'); 
}

You could also exclude the files that you don’t want to be counted against your Big PR threshold. For example, if you want to exclude the mock files:

const linesCount = await danger.git.linesOfCode("**/*");
// exclude mock files
const excludeLinesCount = await danger.git.linesOfCode("**/*mock*");
const totalLinesCount = linesCount - excludeLinesCount;

if (totalLinesCount > bigPRThreshold) { 
  warn("Big PR, break down into smaller PRs."); 
}

Check for markdown typos

Typos in our docs lead to“broken windows”in our codebase. We shouldn’t ignore them. The good news is there is already a Danger JS extension for checking typos:danger-plugin-spellcheck.

Here’s a sample usage from the GitHub repo:

// dangerfile.js
import spellcheck from 'danger-plugin-spellcheck'

spellcheck({ 
  ignore: ['Nachoz', 'Tacoz'], 
  ignoreFiles: ['README.md'] 
})

Positive rules, e.g., complement the PR author

Danger JS is not just about the “you forgot to do this” type of rule. You can use it to compliment your teammates in their PRs by adding positive rules in your dangerfile.

For example: Compliment the author when they have removed more code than added.This doesn’t mean that your team should avoid adding code. It’s a way to encourage the team to remove the redundant code. Less code, less maintenance.

You can do this with a simpleifblock in Danger JS.

if (danger.github.pr.deletions > danger.github.pr.additions) { 
  message(`:thumbsup: You removed more code than added!`); 
}

We can refer to more examples of these positive rules inDanger JS’s doc about positive rules.

Check for missing tests

How many times do we write and receive this PR feedback?

Should we add a test for this?

or

Please add a test for this.

Probably a lot. We don’t have to do this again.

Here are the steps involved when we review tests in a PR:

1. Check if the new function or method needs a test.
2. Check if the test exists — pattern matching.
3. Make sure that the test covers the paths that we need to test.

We can use Danger JS to automate Step 2. We don’t even have to automate Steps 1 and 3. Automating Step 2 is more straightforward and can save us a lot of time.

Checking for missing tests is not the same as calculating test coverage, where the build fails if the PR doesn’t meet a specified coverage threshold.

We don’t calculate the test coverage off-hand when we check for tests during code reviews. We instead look for patterns. For example: If you are reviewing a PR in Golang:

• If there is a new functionGetUsers(), is there an associated test calledTest_GetUsers()?
• IfTest_GetUsers()is missing, leave feedback to “Add the missing test.”

We can programmatically execute the above pattern-matching logic using Danger JS. This will only work if we follow a naming convention.

Steps involved in our Danger JS code to look for tests within a PR:

1. Get the PR diff.
2. Check if there is a new function added. We could use regex for this.
3. If there is a new function, find if there is an associated test for the function. For example, if we found the newly added function in the PR —GetUsers(). Then, we should look for the test:Test_GetUsers().
4. Leave a comment on the PR ifTest_GetUsers()doesn’t exist.

The pseudo code will look like the following:

// Check for missing tests in added functions in a PR:
addedFunctionNames = []
diffs = []
modifiedFiles = getModifiedFiles()
modifiedTestFiles = getModifiedTestFiles()
diffs = modifiedFiles.map(m => getDiffAdded(m))
testDiffs = modifiedTestFiles.map(m => getDiffAdded(m))
for (diff in diffs) { 
  parsedFunctionName = parseFunctionNames(diff) 
  // check if the pattern matches 
  testsFound = testDiffs.filter(diff => diff.includes(`test_${parsedFunctionName}`)) 
  if (testsFound.length === 0) { 
    warn(`tests missing for: ${parsedFunctionName}`) 
  } 
}

Setting up Danger JS in Google Cloudbuild is straightforward. Check myprevious blog post to learn more.