Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Danger #14956

Closed
wants to merge 11 commits into from
Closed

[WIP] Danger #14956

wants to merge 11 commits into from

Conversation

hramos
Copy link
Contributor

@hramos hramos commented Jul 11, 2017

Testing Danger support in CI. Continuation of #14953, which Circle stopped building.

Test Plan

Update your node modules first: npm install

Test WIP, package.json warnings

npm run danger pr https://github.com/facebook/react-native/pull/14951
Verify output. This PR should trigger a WIP warning, as well as a package.json warning:

> [email protected] danger /Users/hramos/git/react-native
> node ./node_modules/.bin/danger "pr" "https://github.com/facebook/react-native/pull/14951"

{
  fails: [],
  warnings: [
    {
      message: ":construction_worker: Work In Progress - <i>Do not merge yet.</i>"
    }, 
    {
      message: ":lock: Changes were made to package.json - <i>This will require a manual import. Once approved, a Facebook employee should import the PR, then run `yarn add` for any new packages.</i>"
    }
  ],
  messages: [],
  markdowns: ["This PR requires attention from the @facebook/react-native team."]
}

Test plan warnings

npm run danger pr https://github.com/facebook/react-native/pull/14946

Verify output. This PR should trigger a warning against the lack of a test plan (note that the PR does have a test plan, but it does not title it as such):

{
  fails: [],
  warnings: [
    {
      message: ":clipboard: Test Plan - <i>This PR appears to be missing a Test Plan</i>"
    }
  ],
  messages: [],
  markdowns: []
}

Warn when a change to the Getting Started guide is missing a test plan

npm run danger pr https://github.com/facebook/react-native/pull/13186

Should warn against a missing test plan:

{
  fails: [],
  warnings: [
    {
      message: ":clipboard: Test Plan - <i>This PR appears to be missing a Test Plan.</i>"
    }
  ],
  messages: [],
  markdowns: [":page_facing_up: Thanks for your contribution to the docs!"]
}

Flag PRs by core contributors

If the author is able to issue bot commands, we reasonably assume that this is coming from an established core contributor. Their PRs will be flagged for expedited review:

npm run danger pr https://github.com/facebook/react-native/pull/14895

{
  fails: [],
  warnings: [
    {
      message: ":clipboard: Test Plan - <i>This PR appears to be missing a Test Plan.</i>"
    }
  ],
  messages: [],
  markdowns: ["This PR has been submitted by a core contributor. Notifying @facebook/react-native."]
}

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jul 11, 2017
@@ -0,0 +1 @@
{}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not merge until we can remove this. This is only necessary until a new release of Jest is cut. See danger/danger-js#261

@hramos hramos closed this Jul 12, 2017
@hramos hramos mentioned this pull request Jul 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants