From 740bec7941c1bc8b05e01d29f48ddf4fa5996a1e Mon Sep 17 00:00:00 2001 From: Hector Ramos Date: Tue, 27 Jun 2017 14:46:21 -0700 Subject: [PATCH 01/10] Adds Danger support --- .babelrc | 1 + Dangerfile.js | 29 +++++++++++++++++++++++++++++ package.json | 4 +++- 3 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 .babelrc create mode 100644 Dangerfile.js diff --git a/.babelrc b/.babelrc new file mode 100644 index 00000000000000..0967ef424bce67 --- /dev/null +++ b/.babelrc @@ -0,0 +1 @@ +{} diff --git a/Dangerfile.js b/Dangerfile.js new file mode 100644 index 00000000000000..8ea58537f07484 --- /dev/null +++ b/Dangerfile.js @@ -0,0 +1,29 @@ +/** + * Copyright (c) 2016-present, Facebook, Inc. All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +const fs = require('fs'); +const includes = require('lodash.includes'); + +// Warns if there are changes to package.json without changes to yarn.lock. +const packageChanged = includes(danger.git.modified_files, 'package.json'); +if (packageChanged) { + const message = 'Changes were made to package.json'; + const idea = 'This will require a manual import by a Facebook employee'; + warn(`${message} - ${idea}`); +} + +// PR linting +if (danger.github.pr.body.length < 10) { + fail("This pull request needs an description.") +} + +if (!danger.github.pr.body.toLowerCase().includes("test plan")) { + const message = 'Test Plan'; + const idea = 'This PR appears to be missing a Test Plan'; + warn(`${message} - ${idea}`); +} diff --git a/package.json b/package.json index 4129ad6c5db69e..1eec3ee116a72d 100644 --- a/package.json +++ b/package.json @@ -126,7 +126,8 @@ "test-android-all": "npm run test-android-build && npm run test-android-run-unit && npm run test-android-run-instrumentation && npm run test-android-run-e2e", "test-android-instrumentation": "npm run test-android-build && npm run test-android-run-instrumentation", "test-android-unit": "npm run test-android-build && npm run test-android-run-unit", - "test-android-e2e": "npm run test-android-build && npm run test-android-run-e2e" + "test-android-e2e": "npm run test-android-build && npm run test-android-run-e2e", + "danger": "node ./danger/node_modules/.bin/danger" }, "bin": { "react-native": "local-cli/wrong-react-native.js" @@ -220,6 +221,7 @@ }, "devDependencies": { "babel-eslint": "^7.2.3", + "danger": "^0.21.2", "eslint": "^3.19.0", "eslint-config-fb-strict": "^20.0.3", "eslint-config-fbjs": "^1.1.1", From 9f9f9948a7f23d601ec7dde81535ca64e6f3c270 Mon Sep 17 00:00:00 2001 From: Hector Ramos Date: Tue, 11 Jul 2017 13:35:40 -0700 Subject: [PATCH 02/10] Additional Dangerfile rules, and actually run in CI. --- Dangerfile.js | 56 +++++++++++++++++++++++++++++++++++++++++++-------- circle.yml | 2 ++ 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/Dangerfile.js b/Dangerfile.js index 8ea58537f07484..3b1eba187b0fa2 100644 --- a/Dangerfile.js +++ b/Dangerfile.js @@ -9,21 +9,61 @@ const fs = require('fs'); const includes = require('lodash.includes'); -// Warns if there are changes to package.json without changes to yarn.lock. -const packageChanged = includes(danger.git.modified_files, 'package.json'); -if (packageChanged) { - const message = 'Changes were made to package.json'; - const idea = 'This will require a manual import by a Facebook employee'; +const isDocsFile = path => includes(path, '/docs/'); +const editsDocs = danger.git.modified_files.filter(isDocsFile).length > 0; +const addsDocs = danger.git.created_files.filter(isDocsFile).length > 0; +if (addsDocs || editsDocs) { + markdown(`:page_facing_up: Thanks for your contribution to the docs!`); +} + +const isBlogFile = path => includes(path, '/blog/'); +const addsBlogPost = danger.git.created_files.filter(isBlogFile).length > 0; +if (addsBlogPost) { + const message = ':memo: Blog post'; + const idea = 'This PR appears to add a new blog post, and may require further review from the React Native team.'; warn(`${message} - ${idea}`); + markdown(`:memo: This PR requires attention from the @facebook/react-native team.`); +} +const editsBlogPost = danger.git.modified_files.filter(isBlogFile).length > 0; +if (editsBlogPost) { + const message = ':memo: Blog post'; + const idea = 'This PR appears to edit an existing blog post, and may require further review from the React Native team.'; + warn(`${message} - ${idea}`); + markdown(`This PR requires attention from the @facebook/react-native team.`); } -// PR linting +// Fails if the description is too short. if (danger.github.pr.body.length < 10) { - fail("This pull request needs an description.") + fail(":grey_question: This pull request needs a description.") +} + +// Warns if the PR title contains [WIP] +const isWIP = includes(danger.github.pr.title, "[WIP]") +if (isWIP) { + const message = ':construction_worker: Work In Progress'; + const idea = 'Do not merge yet.'; + warn(`${message} - ${idea}`); +} + +// Warns if there are changes to package.json, and tags the team. +const packageChanged = includes(danger.git.modified_files, 'package.json'); +if (packageChanged) { + const message = ':lock: Changes were made to package.json'; + const idea = 'This will require a manual import. Once approved, a Facebook employee should import the PR, then run `yarn add` for any new packages.'; + warn(`${message} - ${idea}`); + markdown(`This PR requires attention from the @facebook/react-native team.`); } +// Warns if a test plan is missing. if (!danger.github.pr.body.toLowerCase().includes("test plan")) { - const message = 'Test Plan'; + const message = ':clipboard: Test Plan'; const idea = 'This PR appears to be missing a Test Plan'; warn(`${message} - ${idea}`); } + +// Tags the React Native team is the PR is submitted by a core contributor +const taskforce = fs.readFileSync("bots/IssueCommands.txt").split("\n")[0].split(":")[1]; +const isSubmittedByTaskforce = includes(taskforce, danger.github.pr.user.login); +if (isSubmittedByTaskforce) { + markdown(`This PR has been submitted by a core contributor. Notifying @facebook/react-native.`); +} diff --git a/circle.yml b/circle.yml index 6b36c93d5d224b..0b11489b1d5cb5 100644 --- a/circle.yml +++ b/circle.yml @@ -49,6 +49,8 @@ test: - source scripts/circle-ci-android-setup.sh && waitForAVD override: + # Lint PRs. + - if [[ $CI_PULL_REQUEST ]]; then yarn danger; fi # eslint bot. This GitHub token grants public_repo access scope. - cat <(echo eslint; npm run lint --silent -- --format=json; echo flow; npm run flow --silent -- check --json) | GITHUB_TOKEN="af6ef0d15709bc91d""06a6217a5a826a226fb57b7" CI_USER=$CIRCLE_PROJECT_USERNAME CI_REPO=$CIRCLE_PROJECT_REPONAME PULL_REQUEST_NUMBER=$CIRCLE_PR_NUMBER node bots/code-analysis-bot.js # JS tests for dependencies installed with npm3 From efdedad739fa76dc3743ae5708a355a34ed8d931 Mon Sep 17 00:00:00 2001 From: Hector Ramos Date: Tue, 11 Jul 2017 13:42:53 -0700 Subject: [PATCH 03/10] Force another run in Circle --- circle.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/circle.yml b/circle.yml index 0b11489b1d5cb5..e655a6b1895ed1 100644 --- a/circle.yml +++ b/circle.yml @@ -49,7 +49,7 @@ test: - source scripts/circle-ci-android-setup.sh && waitForAVD override: - # Lint PRs. + # Run Danger against PRs. - if [[ $CI_PULL_REQUEST ]]; then yarn danger; fi # eslint bot. This GitHub token grants public_repo access scope. - cat <(echo eslint; npm run lint --silent -- --format=json; echo flow; npm run flow --silent -- check --json) | GITHUB_TOKEN="af6ef0d15709bc91d""06a6217a5a826a226fb57b7" CI_USER=$CIRCLE_PROJECT_USERNAME CI_REPO=$CIRCLE_PROJECT_REPONAME PULL_REQUEST_NUMBER=$CIRCLE_PR_NUMBER node bots/code-analysis-bot.js From 0e48ea40d5c2e4d0ca01acc30c36b77d2deddcc6 Mon Sep 17 00:00:00 2001 From: Hector Ramos Date: Tue, 11 Jul 2017 13:55:49 -0700 Subject: [PATCH 04/10] Use correct script location. --- circle.yml | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/circle.yml b/circle.yml index e655a6b1895ed1..428740de231c47 100644 --- a/circle.yml +++ b/circle.yml @@ -50,7 +50,7 @@ test: override: # Run Danger against PRs. - - if [[ $CI_PULL_REQUEST ]]; then yarn danger; fi + - if [[ $CI_PULL_REQUEST ]]; then npm run danger; fi # eslint bot. This GitHub token grants public_repo access scope. - cat <(echo eslint; npm run lint --silent -- --format=json; echo flow; npm run flow --silent -- check --json) | GITHUB_TOKEN="af6ef0d15709bc91d""06a6217a5a826a226fb57b7" CI_USER=$CIRCLE_PROJECT_USERNAME CI_REPO=$CIRCLE_PROJECT_REPONAME PULL_REQUEST_NUMBER=$CIRCLE_PR_NUMBER node bots/code-analysis-bot.js # JS tests for dependencies installed with npm3 diff --git a/package.json b/package.json index 1eec3ee116a72d..d0bf87075f64ef 100644 --- a/package.json +++ b/package.json @@ -127,7 +127,7 @@ "test-android-instrumentation": "npm run test-android-build && npm run test-android-run-instrumentation", "test-android-unit": "npm run test-android-build && npm run test-android-run-unit", "test-android-e2e": "npm run test-android-build && npm run test-android-run-e2e", - "danger": "node ./danger/node_modules/.bin/danger" + "danger": "node ./node_modules/.bin/danger" }, "bin": { "react-native": "local-cli/wrong-react-native.js" From b2b63fa3ce38dc248d9721025692a09dafbfa023 Mon Sep 17 00:00:00 2001 From: Hector Ramos Date: Tue, 11 Jul 2017 14:04:43 -0700 Subject: [PATCH 05/10] Force another run --- Dangerfile.js | 1 + 1 file changed, 1 insertion(+) diff --git a/Dangerfile.js b/Dangerfile.js index 3b1eba187b0fa2..39a5063f78067c 100644 --- a/Dangerfile.js +++ b/Dangerfile.js @@ -6,6 +6,7 @@ * of patent rights can be found in the PATENTS file in the same directory. */ +const { danger, fail, markdown, warn } = require('danger'); const fs = require('fs'); const includes = require('lodash.includes'); From dbb1c595c2b35f723f71c8280698c2a5b74823d0 Mon Sep 17 00:00:00 2001 From: Hector Ramos Date: Tue, 11 Jul 2017 14:10:01 -0700 Subject: [PATCH 06/10] Fix parsing of taskforce users. --- Dangerfile.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Dangerfile.js b/Dangerfile.js index 39a5063f78067c..6130331876b00c 100644 --- a/Dangerfile.js +++ b/Dangerfile.js @@ -6,7 +6,7 @@ * of patent rights can be found in the PATENTS file in the same directory. */ -const { danger, fail, markdown, warn } = require('danger'); +// Removed require const fs = require('fs'); const includes = require('lodash.includes'); @@ -35,11 +35,11 @@ if (editsBlogPost) { // Fails if the description is too short. if (danger.github.pr.body.length < 10) { - fail(":grey_question: This pull request needs a description.") + fail(':grey_question: This pull request needs a description.') } // Warns if the PR title contains [WIP] -const isWIP = includes(danger.github.pr.title, "[WIP]") +const isWIP = includes(danger.github.pr.title, '[WIP]') if (isWIP) { const message = ':construction_worker: Work In Progress'; const idea = 'Do not merge yet.'; @@ -56,14 +56,14 @@ if (packageChanged) { } // Warns if a test plan is missing. -if (!danger.github.pr.body.toLowerCase().includes("test plan")) { +if (!danger.github.pr.body.toLowerCase().includes('test plan')) { const message = ':clipboard: Test Plan'; const idea = 'This PR appears to be missing a Test Plan'; warn(`${message} - ${idea}`); } // Tags the React Native team is the PR is submitted by a core contributor -const taskforce = fs.readFileSync("bots/IssueCommands.txt").split("\n")[0].split(":")[1]; +const taskforce = fs.readFileSync('bots/IssueCommands.txt', 'utf8').split('\n')[0].split(':')[1]; const isSubmittedByTaskforce = includes(taskforce, danger.github.pr.user.login); if (isSubmittedByTaskforce) { markdown(`This PR has been submitted by a core contributor. Notifying @facebook/react-native.`); From 8b8afc6d0257643b084b833f37c4b045269db1e5 Mon Sep 17 00:00:00 2001 From: Hector Ramos Date: Tue, 11 Jul 2017 14:36:36 -0700 Subject: [PATCH 07/10] Flag Getting Started edits. --- Dangerfile.js | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/Dangerfile.js b/Dangerfile.js index 6130331876b00c..8d330d542ce26e 100644 --- a/Dangerfile.js +++ b/Dangerfile.js @@ -10,14 +10,15 @@ const fs = require('fs'); const includes = require('lodash.includes'); -const isDocsFile = path => includes(path, '/docs/'); +const isDocsFile = path => includes(path, 'docs/'); const editsDocs = danger.git.modified_files.filter(isDocsFile).length > 0; const addsDocs = danger.git.created_files.filter(isDocsFile).length > 0; +// Note, this does not yet cover edits to the autogenerated docs (e.g. comments within JS source files) if (addsDocs || editsDocs) { markdown(`:page_facing_up: Thanks for your contribution to the docs!`); } -const isBlogFile = path => includes(path, '/blog/'); +const isBlogFile = path => includes(path, 'blog/'); const addsBlogPost = danger.git.created_files.filter(isBlogFile).length > 0; if (addsBlogPost) { const message = ':memo: Blog post'; @@ -56,9 +57,19 @@ if (packageChanged) { } // Warns if a test plan is missing. -if (!danger.github.pr.body.toLowerCase().includes('test plan')) { +const gettingStartedChanged = includes(danger.git.modified_files, 'docs/GettingStarted.md'); +const includesTestPlan = danger.github.pr.body.toLowerCase().includes('test plan'); + +// Warns if a test plan is missing, when editing the Getting Started guide. This page needs to be tested in all its permutations. +if (!includesTestPlan && gettingStartedChanged) { + const message = ':clipboard: Test Plan'; + const idea = 'This PR appears to be missing a Test Plan.'; + warn(`${message} - ${idea}`); +} +// Doc edits rarely require a test plan. We'll trust the reviewer to push back if one is needed. +if (!includesTestPlan && !editsDocs) { const message = ':clipboard: Test Plan'; - const idea = 'This PR appears to be missing a Test Plan'; + const idea = 'This PR appears to be missing a Test Plan.'; warn(`${message} - ${idea}`); } From 337b7b99886f8b22ea4600d1a01b9b172f3650d7 Mon Sep 17 00:00:00 2001 From: Hector Ramos Date: Tue, 11 Jul 2017 14:46:54 -0700 Subject: [PATCH 08/10] Add comments --- Dangerfile.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Dangerfile.js b/Dangerfile.js index 8d330d542ce26e..55fb0ff8f7c70e 100644 --- a/Dangerfile.js +++ b/Dangerfile.js @@ -13,12 +13,14 @@ const includes = require('lodash.includes'); const isDocsFile = path => includes(path, 'docs/'); const editsDocs = danger.git.modified_files.filter(isDocsFile).length > 0; const addsDocs = danger.git.created_files.filter(isDocsFile).length > 0; -// Note, this does not yet cover edits to the autogenerated docs (e.g. comments within JS source files) if (addsDocs || editsDocs) { + // Note, this does not yet cover edits to the autogenerated docs (e.g. comments within JS source files) markdown(`:page_facing_up: Thanks for your contribution to the docs!`); } const isBlogFile = path => includes(path, 'blog/'); + +// Flags new blog posts. const addsBlogPost = danger.git.created_files.filter(isBlogFile).length > 0; if (addsBlogPost) { const message = ':memo: Blog post'; @@ -26,6 +28,8 @@ if (addsBlogPost) { warn(`${message} - ${idea}`); markdown(`:memo: This PR requires attention from the @facebook/react-native team.`); } + +// Flags edits to blog posts const editsBlogPost = danger.git.modified_files.filter(isBlogFile).length > 0; if (editsBlogPost) { const message = ':memo: Blog post'; From b91940f762a004292fd1f510d96769872d58b535 Mon Sep 17 00:00:00 2001 From: Hector Ramos Date: Tue, 11 Jul 2017 15:16:50 -0700 Subject: [PATCH 09/10] Pass on API token. --- circle.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/circle.yml b/circle.yml index c2cad0c04033e4..35118212e41bcb 100644 --- a/circle.yml +++ b/circle.yml @@ -50,7 +50,7 @@ test: override: # Run Danger against PRs. - - if [[ $CI_PULL_REQUEST ]]; then npm run danger; fi + - if [[ $CI_PULL_REQUEST ]]; then DANGER_GITHUB_API_TOKEN=$DANGER_GITHUB_API_TOKEN npm run danger; fi # eslint bot. This GitHub token grants public_repo access scope. - cat <(echo eslint; npm run lint --silent -- --format=json; echo flow; npm run flow --silent -- check --json) | GITHUB_TOKEN="af6ef0d15709bc91d""06a6217a5a826a226fb57b7" CI_USER=$CIRCLE_PROJECT_USERNAME CI_REPO=$CIRCLE_PROJECT_REPONAME PULL_REQUEST_NUMBER=$CIRCLE_PR_NUMBER node bots/code-analysis-bot.js - npm run lint From 68fd826ba1c61a0082d83362200daf596b950025 Mon Sep 17 00:00:00 2001 From: Hector Ramos Date: Tue, 11 Jul 2017 16:18:26 -0700 Subject: [PATCH 10/10] Add token in plain text - it's safe, as this is from a public bot. --- circle.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/circle.yml b/circle.yml index 35118212e41bcb..5eedefe2c93373 100644 --- a/circle.yml +++ b/circle.yml @@ -50,7 +50,7 @@ test: override: # Run Danger against PRs. - - if [[ $CI_PULL_REQUEST ]]; then DANGER_GITHUB_API_TOKEN=$DANGER_GITHUB_API_TOKEN npm run danger; fi + - if [[ $CI_PULL_REQUEST ]]; then DANGER_GITHUB_API_TOKEN="5fc403da51e5e05666e1e3ae6380190178a1cdb3" npm run danger; fi # eslint bot. This GitHub token grants public_repo access scope. - cat <(echo eslint; npm run lint --silent -- --format=json; echo flow; npm run flow --silent -- check --json) | GITHUB_TOKEN="af6ef0d15709bc91d""06a6217a5a826a226fb57b7" CI_USER=$CIRCLE_PROJECT_USERNAME CI_REPO=$CIRCLE_PROJECT_REPONAME PULL_REQUEST_NUMBER=$CIRCLE_PR_NUMBER node bots/code-analysis-bot.js - npm run lint