Skip to content

Commit ee99dea

Browse files
LeSeulArtichauttshepang
authored andcommitted
Clean up 'Contributing to Rust - Pull Requests'
1 parent a85aed0 commit ee99dea

File tree

1 file changed

+54
-36
lines changed

1 file changed

+54
-36
lines changed

src/contributing.md

+54-36
Original file line numberDiff line numberDiff line change
@@ -67,41 +67,15 @@ in the appropriate provided template.
6767

6868
## Pull Requests
6969

70-
Pull requests are the primary mechanism we use to change Rust. GitHub itself
71-
has some [great documentation][about-pull-requests] on using the Pull Request feature.
72-
We use the "fork and pull" model [described here][development-models], where
73-
contributors push changes to their personal fork and create pull requests to
70+
Pull requests (or PRs for short) are the primary mechanism we use to change Rust.
71+
GitHub itself has some [great documentation][about-pull-requests] on using the
72+
Pull Request feature. We use the "fork and pull" model [described here][development-models],
73+
where contributors push changes to their personal fork and create pull requests to
7474
bring those changes into the source repository.
7575

7676
[about-pull-requests]: https://help.github.com/articles/about-pull-requests/
7777
[development-models]: https://help.github.com/articles/about-collaborative-development-models/
7878

79-
Please make pull requests against the `master` branch.
80-
81-
Rust follows a _no merge-commit policy_, meaning, when you encounter merge
82-
conflicts you are expected to always rebase instead of merge. E.g. always use
83-
rebase when bringing the latest changes from the master branch to your feature
84-
branch. Also, please make sure that fixup commits are squashed into other
85-
related commits with meaningful commit messages.
86-
87-
GitHub allows [closing issues using keywords][closing-keywords]. This feature
88-
should be used to keep the issue tracker tidy. However, it is generally preferred
89-
to put the "closes #123" text in the PR description rather than the issue commit;
90-
particularly during rebasing, citing the issue number in the commit can "spam"
91-
the issue in question.
92-
93-
[closing-keywords]: https://help.github.com/en/articles/closing-issues-using-keywords
94-
95-
Please make sure your pull request is in compliance with Rust's style
96-
guidelines by running
97-
98-
$ ./x.py test tidy
99-
100-
Make this check before every pull request (and every new commit in a pull
101-
request); you can add [git hooks](https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks)
102-
before every push to make sure you never forget to make this check. The
103-
CI will also run tidy and will fail if tidy fails.
104-
10579
All pull requests are reviewed by another person. We have a bot,
10680
[@rust-highfive][rust-highfive], that will automatically assign a random person
10781
to review your request.
@@ -116,32 +90,76 @@ make a documentation change, add
11690
to the end of the pull request description, and [@rust-highfive][rust-highfive] will assign
11791
[@steveklabnik][steveklabnik] instead of a random person. This is entirely optional.
11892

93+
In addition to being reviewed by a human, pull requests are automatically tested
94+
thanks to continuous integration (CI). Basically, every time you open and update
95+
a pull request, the CI builds the compiler and tests it against the
96+
[compiler test suite][rctd], and also performs other tests such as checking that
97+
your pull request is in compliance with Rust's style guidelines.
98+
99+
Running continuous integration tests allows PR authors to catch mistakes early
100+
without going through a first review cycle, and also helps reviewers stay aware
101+
of the status of a particular pull request.
102+
103+
Rust has plenty of CI capacity, and you should never have to worry about wasting
104+
computational resources each time you push a change. It is also perfectly fine
105+
(and even encouraged!) to use the CI to test your changes if it can help your
106+
productivity, e.g. if your machine is not very powerful.
107+
119108
After someone has reviewed your pull request, they will leave an annotation
120109
on the pull request with an `r+`. It will look something like this:
121110

122111
@bors r+
123112

124113
This tells [@bors], our lovable integration bot, that your pull request has
125114
been approved. The PR then enters the [merge queue][merge-queue], where [@bors]
126-
will run all the tests on every platform we support. If it all works out,
115+
will run *all* the tests on *every* platform we support. If it all works out,
127116
[@bors] will merge your code into `master` and close the pull request.
128117

129118
Depending on the scale of the change, you may see a slightly different form of `r+`:
130119

131120
@bors r+ rollup
132121

133-
The additional `rollup` tells [@bors] that this change is eligible for to be
134-
"rolled up". Changes that are rolled up are tested and merged at the same time, to
122+
The additional `rollup` tells [@bors] that this change should always be "rolled up".
123+
Changes that are rolled up are tested and merged alongside other PRs, to
135124
speed the process up. Typically only small changes that are expected not to conflict
136-
with one another are rolled up.
125+
with one another are marked as "always roll up".
137126

138127
[rust-highfive]: https://github.com/rust-highfive
139128
[steveklabnik]: https://github.com/steveklabnik
140129
[@bors]: https://github.com/bors
141130
[merge-queue]: https://buildbot2.rust-lang.org/homu/queue/rust
142131

143-
Speaking of tests, Rust has a comprehensive test suite. More information about
144-
it can be found [here][rctd].
132+
### Opening a PR
133+
134+
You are now ready to file a pull request? Great! Here are a few points you
135+
should be aware of.
136+
137+
All pull requests should be filed against the `master` branch, except in very
138+
particular scenarios. Unless you know for sure that you should target another
139+
branch, `master` will be the right choice.
140+
141+
Make sure your pull request is in compliance with Rust's style guidelines by running
142+
143+
$ ./x.py test tidy
144+
145+
We recommand to make this check before every pull request (and every new commit
146+
in a pull request); you can add [git hooks](https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks)
147+
before every push to make sure you never forget to make this check. The
148+
CI will also run tidy and will fail if tidy fails.
149+
150+
Rust follows a _no merge-commit policy_, meaning, when you encounter merge
151+
conflicts you are expected to always rebase instead of merging. E.g. always use
152+
rebase when bringing the latest changes from the master branch to your feature
153+
branch. Also, please make sure that fixup commits are squashed into other
154+
related commits with meaningful commit messages.
155+
156+
GitHub allows [closing issues using keywords][closing-keywords]. This feature
157+
should be used to keep the issue tracker tidy. However, it is generally preferred
158+
to put the "closes #123" text in the PR description rather than the issue commit;
159+
particularly during rebasing, citing the issue number in the commit can "spam"
160+
the issue in question.
161+
162+
[closing-keywords]: https://help.github.com/en/articles/closing-issues-using-keywords
145163

146164
### External Dependencies (subtree)
147165

0 commit comments

Comments
 (0)