Skip to content

contributing guide: mention expectations around force pushes and squashing #3998

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 28, 2024
Merged

contributing guide: mention expectations around force pushes and squashing #3998

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,25 @@ for a list of Miri maintainers.

[Rust Zulip]: https://rust-lang.zulipchat.com

### Pull review process

When you get a review, please take care of the requested changes in new commits. Do not amend
existing commits. Generally avoid force-pushing. The only time you should force push is when there
is a conflict with the master branch (in that case you should rebase across master, not merge), and
all the way at the end of the review process when the reviewer tells you that the PR is done and you
should squash the commits. For the latter case, use `git rebase --keep-base ...` to squash without
changing the base commit your PR branches off of. Use your own judgment and the reviewer's guidance
to decide whether the PR should be squashed into a single commit or multiple logically separate
commits. (All this is to work around the fact that Github is quite bad at dealing with force pushes
and does not support `git range-diff`. Maybe one day Github will be good at git and then life can
become easier.)

Most PRs bounce back and forth between the reviewer and the author several times, so it is good to
keep track of who is expected to take the next step. We are using the `S-waiting-for-review` and
`S-waiting-for-author` labels for that. If a reviewer asked you to do some changes and you think
they are all taken care of, post a comment saying `@rustbot ready` to mark a PR as ready for the
next round of review.

### Larger-scale contributions

If you are thinking about making a larger-scale contribution -- in particular anything that needs
Expand Down Expand Up @@ -45,14 +64,6 @@ process for such contributions:
This process is largely informal, and its primary goal is to more clearly communicate expectations.
Please get in touch with us if you have any questions!

### Managing the review state

Most PRs bounce back and forth between the reviewer and the author several times, so it is good to
keep track of who is expected to take the next step. We are using the `S-waiting-for-review` and
`S-waiting-for-author` labels for that. If a reviewer asked you to do some changes and you think
they are all taken care of, post a comment saying `@rustbot ready` to mark a PR as ready for the
next round of review.

## Preparing the build environment

Miri heavily relies on internal and unstable rustc interfaces to execute MIR,
Expand Down