Skip to content

Commit ae775af

Browse files
blakeffacebook-github-bot
authored andcommitted
Danger shouldn't warn for package.json changes (#48148)
Summary: Someone always has to merge in from Meta, so this is just noise. Refactored some of this older code. Changelog: [Internal] Pull Request resolved: #48148 Test Plan: This PR, but this doesn't build a lot of confidence: {F1973526183} Reviewed By: rubennorte Differential Revision: D66876722 Pulled By: blakef fbshipit-source-id: 52e1f15577f8f057ceee9427af65df43f152bffa
1 parent c54ba09 commit ae775af

File tree

1 file changed

+27
-23
lines changed

1 file changed

+27
-23
lines changed

packages/react-native-bots/dangerfile.js

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,31 @@
88
*/
99

1010
'use strict';
11-
const {danger, fail, /*message,*/ warn} = require('danger');
12-
const includes = require('lodash.includes');
11+
const {danger, fail, warn} = require('danger');
1312

14-
const isFromPhabricator =
15-
danger.github.pr.body &&
16-
danger.github.pr.body.toLowerCase().includes('differential revision:');
13+
const body = danger.github.pr.body?.toLowerCase() ?? '';
14+
15+
function body_contains(...text) {
16+
for (const matcher of text) {
17+
if (body.includes(matcher)) {
18+
return true;
19+
}
20+
}
21+
return false;
22+
}
23+
24+
const isFromPhabricator = body_contains('differential revision:');
1725

1826
// Provides advice if a summary section is missing, or body is too short
19-
const includesSummary =
20-
danger.github.pr.body &&
21-
danger.github.pr.body.toLowerCase().includes('## summary');
22-
if (!danger.github.pr.body || danger.github.pr.body.length < 50) {
27+
const includesSummary = body_contains('## summary', 'summary:');
28+
29+
const hasNoUsefulBody =
30+
!danger.github.pr.body || danger.github.pr.body.length < 50;
31+
const hasTooShortAHumanSummary =
32+
!includesSummary && body.split('\n').length <= 2 && !isFromPhabricator;
33+
if (hasNoUsefulBody) {
2334
fail(':grey_question: This pull request needs a description.');
24-
} else if (!includesSummary && !isFromPhabricator) {
35+
} else if (hasTooShortAHumanSummary) {
2536
// PRs from Phabricator always includes the Summary by default.
2637
const title = ':clipboard: Missing Summary';
2738
const idea =
@@ -31,20 +42,13 @@ if (!danger.github.pr.body || danger.github.pr.body.length < 50) {
3142
warn(`${title} - <i>${idea}</i>`);
3243
}
3344

34-
// Warns if there are changes to package.json, and tags the team.
35-
const packageChanged = includes(danger.git.modified_files, 'package.json');
36-
if (packageChanged) {
37-
const title = ':lock: package.json';
38-
const idea =
39-
'Changes were made to package.json. ' +
40-
'This will require a manual import by a Facebook employee.';
41-
warn(`${title} - <i>${idea}</i>`);
42-
}
43-
4445
// Provides advice if a test plan is missing.
45-
const includesTestPlan =
46-
danger.github.pr.body &&
47-
danger.github.pr.body.toLowerCase().includes('## test plan');
46+
const includesTestPlan = body_contains(
47+
'## test plan',
48+
'test plan:',
49+
'tests:',
50+
'test:',
51+
);
4852
if (!includesTestPlan && !isFromPhabricator) {
4953
// PRs from Phabricator never exports the Test Plan so let's disable this check.
5054
const title = ':clipboard: Missing Test Plan';

0 commit comments

Comments
 (0)