Skip to content

Commit 5273553

Browse files
cortinicofacebook-github-bot
authored andcommitted
Make Danger Phabricator-friendly. (#32642)
Summary: Pull Request resolved: #32642 This Diff adapts the Dangerfile to be a bit more friendly for PRs that are exported from Phabricator. Specifically, I'm disabling the check for Test Plan as those are not exported externally. The check for Summary and Changelog are instead adapted to work with the format exported for Phabricator. Changelog: [Internal] [Changed] - Make Danger Phabricator-friendly Reviewed By: yungsters Differential Revision: D32594622 fbshipit-source-id: e31f29defdd926a267cecc9efa6d63de2e580f43
1 parent 2162bce commit 5273553

1 file changed

Lines changed: 11 additions & 3 deletions

File tree

bots/dangerfile.js

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,18 @@
1212
const {danger, fail, message, warn} = require('danger');
1313
const includes = require('lodash.includes');
1414

15+
const isFromPhabricator =
16+
danger.github.pr.body &&
17+
danger.github.pr.body.toLowerCase().includes('differential revision:');
18+
1519
// Provides advice if a summary section is missing, or body is too short
1620
const includesSummary =
1721
danger.github.pr.body &&
1822
danger.github.pr.body.toLowerCase().includes('## summary');
1923
if (!danger.github.pr.body || danger.github.pr.body.length < 50) {
2024
fail(':grey_question: This pull request needs a description.');
21-
} else if (!includesSummary) {
25+
} else if (!includesSummary && !isFromPhabricator) {
26+
// PRs from Phabricator always includes the Summary by default.
2227
const title = ':clipboard: Missing Summary';
2328
const idea =
2429
'Can you add a Summary? ' +
@@ -41,7 +46,8 @@ if (packageChanged) {
4146
const includesTestPlan =
4247
danger.github.pr.body &&
4348
danger.github.pr.body.toLowerCase().includes('## test plan');
44-
if (!includesTestPlan) {
49+
if (!includesTestPlan && !isFromPhabricator) {
50+
// PRs from Phabricator never exports the Test Plan so let's disable this check.
4551
const title = ':clipboard: Missing Test Plan';
4652
const idea =
4753
'Can you add a Test Plan? ' +
@@ -57,7 +63,9 @@ const internalChangelogRegex = /\[\s?(INTERNAL)\s?\].*/gi;
5763
const includesChangelog =
5864
danger.github.pr.body &&
5965
(danger.github.pr.body.toLowerCase().includes('## changelog') ||
60-
danger.github.pr.body.toLowerCase().includes('release notes'));
66+
danger.github.pr.body.toLowerCase().includes('release notes') ||
67+
// PR exports from Phabricator have a `Changelog:` entry for the changelog.
68+
danger.github.pr.body.toLowerCase().includes('changelog:'));
6169
const correctlyFormattedChangelog = changelogRegex.test(danger.github.pr.body);
6270
const containsInternalChangelog = internalChangelogRegex.test(
6371
danger.github.pr.body,

0 commit comments

Comments
 (0)