Add Flow reminder to PR template #8805

Merged
merged 3 commits into from Jan 17, 2017

Projects

None yet

4 participants

@jjcross
Contributor
jjcross commented Jan 16, 2017

Since CircleCI fails on Flow errors, I think it would be helpful to add a reminder to npm run flow before submitting a PR.

Formatting was inspired by the Draft.js PR template.

@gaearon
Member
gaearon commented Jan 16, 2017

We also need to add this step:

  • If you added or removed any tests, run ./scripts/fiber/record-tests before submitting the pull request, and commit the resulting changes.

Could you add this too, and do it both in PR template and here?

docs/contributing/how-to-contribute.md
-5. Make sure your code lints (`npm run lint`).
-6. Run the [Flow](https://flowtype.org/) typechecks (`npm run flow`).
-7. If you haven't already, complete the CLA.
+3. If you added or removed any tests, run ./scripts/fiber/record-tests before submitting the pull request, and commit the resulting changes.
@gaearon
gaearon Jan 17, 2017 Member

Please enclose the command in backticks.

@gaearon
gaearon Jan 17, 2017 Member

Also, this step should probably come the last before CLA. It should be done after everything else is ready.

@jjcross
jjcross Jan 17, 2017 Contributor

That makes more sense, I've made the changes!

@gaearon gaearon merged commit bfd5b18 into facebook:master Jan 17, 2017

1 check was pending

ci/circleci Your tests are queued behind your running builds
Details
@gaearon
Member
gaearon commented Jan 17, 2017

Thanks!

@aweary aweary added a commit that referenced this pull request Jan 18, 2017
@jjcross @aweary jjcross + aweary Add Flow reminder to PR template (#8805)
* Added flow to PR template

* Added record-tests step to PR template and contribution docs

* Updated order of PR checks

(cherry picked from commit bfd5b18)
5446c47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment