| # PR lifecycle | |
| ## Before review | |
| ### Opening a PR | |
| You are now ready to file a pull request (PR)? | |
| Great! | |
| Here are a few points you should be aware of. | |
| All pull requests should be filed against the `main` branch, | |
| unless you know for sure that you should target a different branch. | |
| Run some style checks before you submit the PR: | |
| ./x test tidy --bless | |
| We recommend to make this check before every pull request (and every new commit in a pull request); | |
| you can add [git hooks] before every push to make sure you never forget to make this check. | |
| The CI will also run tidy and will fail if tidy fails. | |
| ### PR description | |
| GitHub allows [closing issues using keywords][closing-keywords]. | |
| This feature should be used to keep the issue tracker tidy. | |
| However, it is generally preferred | |
| to put the "closes #123" text in the PR description rather than the commit message; | |
| particularly during rebasing, citing the issue number in the commit can "spam" | |
| the issue in question. | |
| However, if your PR fixes a stable-to-beta or stable-to-stable regression and has | |
| been accepted for a beta and/or stable backport (i.e., it is marked `beta-accepted` | |
| and/or `stable-accepted`), please do *not* use any such keywords since we don't | |
| want the corresponding issue to get auto-closed once the fix lands on `main`. | |
| Please update the PR description while still mentioning the issue somewhere. | |
| For example, you could write `Fixes (after beta backport) #NNN.`. | |
| [labeling]: ./rustbot.md#issue-relabeling | |
| [closing-keywords]: https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue | |
| ### CI | |
| In addition to being reviewed by a human, pull requests are automatically tested, | |
| thanks to continuous integration (CI). | |
| Basically, every time you open and update | |
| a pull request, CI builds the compiler and tests it against the | |
| [compiler test suite], and also performs other tests such as checking that | |
| your pull request is in compliance with Rust's style guidelines. | |
| Running continuous integration tests allows PR authors to catch mistakes early | |
| without going through a first review cycle, and also helps reviewers stay aware | |
| of the status of a particular pull request. | |
| Rust has plenty of CI capacity, and you should never have to worry about wasting | |
| computational resources each time you push a change. | |
| It is also perfectly fine | |
| (and even encouraged!) to use the CI to test your changes if it can help your productivity. | |
| In particular, we don't recommend running the full `./x test` suite locally, | |
| since it takes a very long time to execute. | |
| See the [Testing with CI] chapter for using Rust's CI to test your changes. | |
| [Testing with CI]: tests/ci.md#testing-with-ci | |
| ## PR review | |
| ### r? | |
| Your PR will be automatically assigned a reviewer. | |
| You can override the reviewer using `r? @username`. | |
| See [PR assignment](https://forge.rust-lang.org/triagebot/pr-assignment.html#usage) for details. | |
| ### Rebasing | |
| Rust follows a _no merge-commit policy_, | |
| meaning that when you encounter merge conflicts, | |
| you are expected to always rebase instead of merging. | |
| For example, | |
| always use rebase when bringing the latest changes from the `main` branch to your feature branch. | |
| If your PR contains merge commits, it will get marked as `has-merge-commits`. | |
| Once you have removed the merge commits, e.g., through an interactive rebase, you | |
| should remove the label again: | |
| @rustbot label -has-merge-commits | |
| See [this chapter][labeling] for more details. | |
| If you encounter merge conflicts or when a reviewer asks you to perform some | |
| changes, your PR will get marked as `S-waiting-on-author`. | |
| When you resolve them, you should use `@rustbot` to mark it as `S-waiting-on-review`: | |
| @rustbot ready | |
| ### Keeping your branch up-to-date | |
| The CI in rust-lang/rust applies your patches directly against current `main`, | |
| not against the commit your branch is based on. | |
| This can lead to unexpected failures | |
| if your branch is outdated, even when there are no explicit merge conflicts. | |
| Update your branch only when needed: when you have merge conflicts, upstream CI is broken and blocking your green PR, or a maintainer requests it. | |
| Avoid updating an already-green PR under review unless necessary. | |
| During review, make incremental commits to address feedback. | |
| Prefer to squash or rebase only at the end, or when a reviewer requests it. | |
| When updating, use `git push --force-with-lease` and leave a brief comment explaining what changed. | |
| Some repos prefer merging from `upstream/main` instead of rebasing; | |
| follow the project's conventions. | |
| See [keeping things up to date](git.md#keeping-things-up-to-date) for detailed instructions. | |
| After rebasing, it's recommended to [run the relevant tests locally](tests/intro.md) to catch any issues before CI runs. | |
| ### Waiting for reviews | |
| > NOTE | |
| > | |
| > Pull request reviewers are often working at capacity, | |
| > and many of them are contributing on a volunteer basis. | |
| > In order to minimize review delays, | |
| > pull request authors and assigned reviewers should ensure that the review label | |
| > (`S-waiting-on-review` and `S-waiting-on-author`) stays updated, | |
| > invoking these commands when appropriate: | |
| > | |
| > - `@rustbot author`: | |
| > the review is finished, | |
| > and PR author should check the comments and take action accordingly. | |
| > | |
| > - `@rustbot ready`: | |
| > the author is ready for a review, | |
| > and this PR will be queued again in the reviewer's queue. | |
| Please note that the reviewers are humans, who for the most part work on `rustc` in their free time. | |
| This means that they can take some time to respond and review your PR. | |
| It also means that reviewers can miss some PRs that are assigned to them. | |
| To try to move PRs forward, the Triage WG regularly goes through all PRs that | |
| are waiting for review and haven't been discussed for at least 2 weeks. | |
| If you don't get a review within 2 weeks, feel free to ask the Triage WG on | |
| Zulip ([#t-release/triage]). | |
| They have knowledge of when to ping, who might be on vacation, etc. | |
| The reviewer may request some changes using the GitHub code review interface. | |
| They may also request special procedures for some PRs. | |
| See [Crater] and [Breaking Changes] chapters for some examples of such procedures. | |
| Feel free to ask questions or discuss things you don't understand or disagree with. | |
| However, recognize that the PR won't be merged unless someone on the Rust team approves it. | |
| If a reviewer leave a comment like `r=me after fixing ...`, that means they approve the PR and | |
| you can merge it with comment with `@bors r=reviewer-github-id`(e.g. `@bors r=eddyb`) to merge it | |
| after fixing trivial issues. | |
| Note that `r=someone` requires permission and bors could say | |
| something like "🔑 Insufficient privileges..." when commenting `r=someone`. | |
| In that case, you have to ask the reviewer to revisit your PR. | |
| There are a couple of things that may happen for some PRs during the review process | |
| - If the change is substantial enough, the reviewer may request an FCP on the PR. | |
| This gives all members of the appropriate team a chance to review the changes. | |
| - If the change may cause breakage, the reviewer may request a [crater] run. | |
| This compiles the compiler with your changes and then attempts to compile all | |
| crates on crates.io with your modified compiler. | |
| This is a great smoke test | |
| to check if you introduced a change to compiler behavior that affects a large | |
| portion of the ecosystem. | |
| - If the diff of your PR is large or the reviewer is busy, your PR may have | |
| some merge conflicts with other PRs that happen to get merged first. | |
| You should fix these merge conflicts using the normal git procedures. | |
| [crater]: ./tests/crater.md | |
| [r?]: https://github.com/rust-lang/rust/pull/78133#issuecomment-712692371 | |
| [#t-release/triage]: https://rust-lang.zulipchat.com/#narrow/stream/242269-t-release.2Ftriage | |
| [Crater]: tests/crater.md | |
| ### r+ | |
| After someone has reviewed your pull request, they will leave an annotation | |
| on the pull request with an `r+`. | |
| It will look something like this: | |
| @bors r+ | |
| This tells [@bors], our lovable integration bot, that your pull request has been approved. | |
| The PR then enters the [merge queue], where [@bors] | |
| will run *all* the tests on *every* platform we support. | |
| Depending on the scale of the change, you may see a slightly different form of `r+`: | |
| @bors r+ rollup | |
| The additional `rollup` tells [@bors] that this change should always be "rolled up". | |
| Changes that are rolled up are tested and merged alongside other PRs, to speed the process up. | |
| Typically, only small changes that are expected not to conflict | |
| with one another are marked as "always roll up". | |
| Be patient; | |
| this can take a while and the queue can sometimes be long. | |
| Also, note that PRs are never merged by hand. | |
| If it all works out, [@bors] will merge your code into `main` and close the pull request. | |
| Your code will be in the next nightly compiler :) | |
| [@rustbot]: https://github.com/rustbot | |
| [@bors]: https://github.com/rust-lang/bors | |
| ## After merge | |
| ### Backports | |
| As for further actions, please keep a sharp look-out for a PR whose title begins with | |
| `[beta]` or `[stable]` and which backports the PR in question. | |
| When that one gets merged, the relevant issue can be closed. | |
| The closing comment should mention all PRs that were involved. | |
| If you don't have the permissions to close the issue, please | |
| leave a comment on the original PR asking the reviewer to close it for you. | |
| ### Reverting a PR | |
| See ["Reverts"](https://forge.rust-lang.org/compiler/reviews.html#reverts) on Forge. | |
| If a PR is large enough that it's hard to revert, it's ok to simply disable the trigger for the | |
| problematic code, as shown in [#128271][#128271]. | |
| For MIR optimizations, we can also use the `-Zunsound-mir-opt` option to gate the mir-opt, as shown | |
| in [#132356][#132356]. | |
| [#128271]: https://github.com/rust-lang/rust/pull/128271 | |
| [#132356]: https://github.com/rust-lang/rust/pull/132356 | |
| [Breaking Changes]: bug-fix-procedure.md | |
| [compiler test suite]: tests/intro.md | |
| [merge queue]: https://bors.rust-lang.org/queue/rust | |
| [git hooks]: https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks | |