Shorten template instructions.
Summary: I've noticed many pull requests are opened with a very short description that fails to explain the motivation behind the PR, and many more forego providing a test plan. With this PR, I am shortening the template in order to provide concise instructions that provide the essential steps above the fold in the "Open a pull request" composer window. - We need people to open a PR against master, not stable. The exact reason for this is not important and providing an explanation takes away from other more important points. - Test plans are essential, but their requirement appears below the fold in the current template. - More PRs could use tests. - Make a point of asking PR authors to follow up on CI test failures. - The composer does not parse Markdown into HTML. Markdown is pretty readable as is, but using reference links instead of inline links should help with readability. I observed that it will only display the first 8 lines of the PR template above the fold. Seeing t Closes https://github.com/facebook/react-native/pull/12958 Differential Revision: D4718294 Pulled By: mkonicek fbshipit-source-id: b19a29e5ed73fb78d09c7de17625b1883590075c
This commit is contained in:
Hector Ramos
Facebook Github Bot
parent
7f4054df76
commit
28bb1960a4
|
|
@ -1,19 +1,32 @@
|
|||
Thanks for submitting a pull request! Please provide enough information so that others can review your pull request:
|
||||
Thanks for submitting a PR! Please read these instructions carefully:
|
||||
|
||||
> **Unless you are a React Native release maintainer and cherry-picking an *existing* commit into a current release, ensure your pull request is targeting the `master` React Native branch.**
|
||||
- [ ] Explain the **motivation** for making this change.
|
||||
- [ ] Provide a **test plan** demonstrating that the code is solid.
|
||||
- [ ] Match the **code formatting** of the rest of the codebase.
|
||||
- [ ] Target the `master` branch, NOT a "stable" branch.
|
||||
|
||||
Explain the **motivation** for making this change. What existing problem does the pull request solve?
|
||||
## Motivation (required)
|
||||
|
||||
Prefer **small pull requests**. These are much easier to review and more likely to get merged. Make sure the PR does only one thing, otherwise please split it.
|
||||
What existing problem does the pull request solve?
|
||||
|
||||
**Test plan (required)**
|
||||
## Test Plan (required)
|
||||
|
||||
Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes UI.
|
||||
A good test plan has the exact commands you ran and their output, provides screenshots or videos if the pull request changes UI or updates the website. See [What is a Test Plan?][1] to learn more.
|
||||
|
||||
Make sure tests pass on both Travis and Circle CI.
|
||||
If you have added code that should be tested, add tests.
|
||||
|
||||
**Code formatting**
|
||||
## Next Steps
|
||||
|
||||
Look around. Match the style of the rest of the codebase. See also the simple [style guide](https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md#style-guide).
|
||||
Sign the [CLA][2], if you haven't already.
|
||||
|
||||
For more info, see the ["Pull Requests" section of our "Contributing" guidelines](https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md#pull-requests).
|
||||
Small pull requests are much easier to review and more likely to get merged. Make sure the PR does only one thing, otherwise please split it.
|
||||
|
||||
Make sure all **tests pass** on both [Travis][3] and [Circle CI][4]. PRs that break tests are unlikely to be merged.
|
||||
|
||||
For more info, see the ["Pull Requests"][5] section of our "Contributing" guidelines.
|
||||
|
||||
[1]: https://medium.com/@martinkonicek/what-is-a-test-plan-8bfc840ec171#.y9lcuqqi9
|
||||
[2]: https://code.facebook.com/cla
|
||||
[3]: https://travis-ci.org/facebook/react-native
|
||||
[4]: http://circleci.com/gh/facebook/react-native
|
||||
[5]: https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md#pull-requests
|
||||
|
|
|
|||
|
|
@ -14,29 +14,30 @@ We will do our best to keep `master` in good shape, with tests passing at all ti
|
|||
|
||||
The core team will be monitoring for pull requests. When we get one, we'll run some Facebook-specific integration tests on it first. From here, we'll need to get another person to sign off on the changes and then merge the pull request. For API changes we may need to fix internal uses, which could cause some delay. We'll do our best to provide updates and feedback throughout the process.
|
||||
|
||||
**Please submit your pull request on the `master` branch**. If the fix is critical and should be included in a stable branch please mention it and it will be cherry picked into it.
|
||||
**Please submit your pull request on the `master` branch**. If the fix is critical and should be included in a stable branch please mention it and it will be cherry picked into it by a project maintainer.
|
||||
|
||||
*Before* submitting a pull request, please make sure the following is done…
|
||||
|
||||
1. Fork the repo and create your branch from `master`.
|
||||
2. **Describe your test plan in your commit.** If you've added code that should be tested, add tests!
|
||||
3. If you've changed APIs, update the documentation.
|
||||
4. If you've updated the docs, verify the website locally and submit screenshots if applicable.
|
||||
2. **Describe your test plan in your commit.**
|
||||
- If you've added code that should be tested, add tests!
|
||||
- If you've changed APIs, update the documentation.
|
||||
- If you've updated the docs, verify the website locally and submit screenshots if applicable.
|
||||
|
||||
```
|
||||
$ cd website
|
||||
$ npm install && npm start
|
||||
go to: http://localhost:8079/react-native/index.html
|
||||
```
|
||||
```
|
||||
$ cd website
|
||||
$ npm install && npm start
|
||||
Open the following in your browser: http://localhost:8079/react-native/index.html
|
||||
```
|
||||
|
||||
5. Add the copyright notice to the top of any new files you've added.
|
||||
6. Ensure tests pass on Travis and Circle CI.
|
||||
7. Make sure your code lints (`node linter.js <files touched>`).
|
||||
8. If you haven't already, sign the [CLA](https://code.facebook.com/cla).
|
||||
9. Squash your commits (`git rebase -i`).
|
||||
one intent alongs with one commit makes it clearer for people to review and easier to understand your intention
|
||||
3. Add the copyright notice to the top of any new files you've added.
|
||||
4. Ensure tests pass on Travis and Circle CI.
|
||||
5. Make sure your code lints (`node linter.js <files touched>`).
|
||||
6. If you haven't already, sign the [CLA](https://code.facebook.com/cla).
|
||||
7. Squash your commits (`git rebase -i`).
|
||||
One intent alongside one commit makes it clearer for people to review and easier to understand your intention.
|
||||
|
||||
Note: It is not necessary to keep clicking `Merge master to your branch` on PR page. You would want to merge master if there are conflicts or tests are failing. The facebook-bot ultimately squashes all commits to a single one before merging your PR.
|
||||
> **Note:** It is not necessary to keep clicking `Merge master to your branch` on the PR page. You would want to merge master if there are conflicts or tests are failing. The Facebook-GitHub-Bot ultimately squashes all commits to a single one before merging your PR.
|
||||
|
||||
#### Copyright Notice for files
|
||||
|
||||
|
|
@ -77,8 +78,8 @@ Facebook has a [bounty program](https://www.facebook.com/whitehat/) for the safe
|
|||
|
||||
## How to Get in Touch
|
||||
|
||||
* [Facebook group](https://www.facebook.com/groups/react.native.community/)
|
||||
* Reactiflux — [#react-native](http://join.reactiflux.com/)
|
||||
* [Facebook](https://www.facebook.com/groups/react.native.community/)
|
||||
* [Twitter](https://www.twitter.com/reactnative)
|
||||
|
||||
## Style Guide
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue