Skip to content

Commit

Permalink
Danger, be nice to PRs. (facebook#23334)
Browse files Browse the repository at this point in the history
Summary:
Changed Danger's config so that it provides advice whenever it finds an issue with the pull request template, instead of posting a warning.

Updated Danger several major versions, from 2 to 7. I worked through any breaking changes, which were minimal (change `yarn danger` to `yarn danger ci`).

Added a flag to have Danger post these messages as GitHub Checks instead of as a comment. This slightly buries Danger's output, as it's no longer posted as a comment, but I believe it integrates more nicely into the GitHub interface.

[GENERAL] [Changed] - GitHub-only change: updated Danger config to be nicer to PRs
Pull Request resolved: facebook#23334

Differential Revision: D14002313

Pulled By: cpojer

fbshipit-source-id: b97ca7b7bd164646b249b7c64b1134306e0f38a8
  • Loading branch information
hramos authored and facebook-github-bot committed Feb 8, 2019
1 parent af41a53 commit d002d30
Show file tree
Hide file tree
Showing 4 changed files with 961 additions and 9 deletions.
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ jobs:
yarn install --non-interactive --cache-folder ~/.cache/yarn
echo -e "\\x1B[36mAnalyzing pull request\\x1B[0m"; \
DANGER_GITHUB_API_TOKEN="$PUBLIC_PULLBOT_GITHUB_TOKEN_A""$PUBLIC_PULLBOT_GITHUB_TOKEN_B" \
yarn danger
yarn danger ci --use-github-checks
when: always
- save-cache: *save-cache-analysis

Expand Down
15 changes: 8 additions & 7 deletions bots/dangerfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@

const includes = require('lodash.includes');

const {danger, fail, warn} = require('danger');
const {danger, fail, message, warn} = require('danger');

// Warns if a summary section is missing, or body is too short
// Provides advice if a summary section is missing, or body is too short
const includesSummary =
danger.github.pr.body &&
danger.github.pr.body.toLowerCase().includes('## summary');
Expand All @@ -28,7 +28,7 @@ if (!danger.github.pr.body || danger.github.pr.body.length < 50) {
'Can you add a Summary? ' +
'To do so, add a "## Summary" section to your PR description. ' +
'This is a good place to explain the motivation for making this change.';
warn(`${title} - <i>${idea}</i>`);
message(`${title} - <i>${idea}</i>`);
}

// Warns if there are changes to package.json, and tags the team.
Expand All @@ -41,7 +41,7 @@ if (packageChanged) {
warn(`${title} - <i>${idea}</i>`);
}

// Warns if a test plan is missing.
// Provides advice if a test plan is missing.
const includesTestPlan =
danger.github.pr.body &&
danger.github.pr.body.toLowerCase().includes('## test plan');
Expand All @@ -51,7 +51,7 @@ if (!includesTestPlan) {
'Can you add a Test Plan? ' +
'To do so, add a "## Test Plan" section to your PR description. ' +
'A Test Plan lets us know how these changes were tested.';
warn(`${title} - <i>${idea}</i>`);
message(`${title} - <i>${idea}</i>`);
}

// Regex looks for given categories, types, a file/framework/component, and a message - broken into 4 capture groups
Expand All @@ -62,6 +62,7 @@ const includesChangelog =
danger.github.pr.body.toLowerCase().includes('release notes'));
const correctlyFormattedChangelog = changelogRegex.test(danger.github.pr.body);

// Provides advice if a changelog is missing
const changelogInstructions =
'A changelog entry has the following format: [`[CATEGORY] [TYPE] - Message`](http://facebook.github.io/react-native/docs/contributing#changelog).';
if (!includesChangelog) {
Expand All @@ -70,11 +71,11 @@ if (!includesChangelog) {
'Can you add a Changelog? ' +
'To do so, add a "## Changelog" section to your PR description. ' +
changelogInstructions;
warn(`${title} - <i>${idea}</i>`);
message(`${title} - <i>${idea}</i>`);
} else if (!correctlyFormattedChangelog) {
const title = ':clipboard: Changelog Format';
const idea = 'Did you include a Changelog? ' + changelogInstructions;
warn(`${title} - <i>${idea}</i>`);
message(`${title} - <i>${idea}</i>`);
}

// Warns if the PR is opened against stable, as commits need to be cherry picked and tagged by a release maintainer.
Expand Down
2 changes: 1 addition & 1 deletion bots/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"danger": "node ./node_modules/.bin/danger"
},
"devDependencies": {
"danger": "^2.1.6",
"danger": "^7.0.8",
"lodash.includes": "^4.3.0",
"minimatch": "^3.0.4"
}
Expand Down
Loading

0 comments on commit d002d30

Please sign in to comment.