Skip to content
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.

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

Change PR strategy from rebase to merge? #1501

Open
RalfJung opened this issue Nov 21, 2023 · 6 comments
Open

Change PR strategy from rebase to merge? #1501

RalfJung opened this issue Nov 21, 2023 · 6 comments

Comments

@RalfJung
Copy link
Member

rust-lang repositories usually "merge" their PRs rather than rebasing them. This repo is an exception. According to @Amanieu, it's been like that for a while and it is unclear why.

IMO we should switch this repo to use "merge" rather than "rebase". However I also just realized that this repo doesn't use bors, is that somehow related to the use of rebases?

@lu-zero
Copy link
Contributor

lu-zero commented Nov 21, 2023

I think it started that way, at least since I started contributing it was setup to favour rebase over merge.

On why some prefer a way or another, it mainly boils down to having bisect tools behave better on the linear history than the tree one, IMHO. I'm not even sure if now git bisect and friends behave nicely or they are still a bit surprising to use.

@bjorn3
Copy link
Member

bjorn3 commented Nov 21, 2023

git bisect --first-parent will bisect individual PR's if you use merge instead of rebase. This works much better if not every commit is guaranteed to build as you don't have to do git bisect skip on every commit that doesn't build.

@RalfJung
Copy link
Member Author

Yeah, bisecting works rust fine on the main rustc repo.

@BurntSushi
Copy link
Member

BurntSushi commented Nov 21, 2023

This was almost certainly my doing. I started this project in the beforetime, when the Rust project had much less process and structure and tooling. I believe it was originally created at http://github.com/BurntSushi/stdsimd and then moved into rust-lang. (The redirect is still there!)

This also explains why it was set to rebase, because I use rebase for all of my projects. I try to treat source history like I treat source code. That is, that it should be cultivated and optimized for humans looking to understand the logical transformations that the source code has undergone. I generally find merge commits to be contrary to this goal.

Of course, with all that said, I haven't been involved with this project for a long time. So if it makes sense to the current maintainers, then I'm totally in favor of changing it to match most of how the rest of the project operates. But if you want to stick on rebase and are looking for precedent, rust-lang/regex has used rebase for many years now and there are no plans to change that.

@RalfJung
Copy link
Member Author

This also explains why it was set to rebase, because I use rebase for all of my projects. I try to treat source history like I treat source code. That is, that it should be cultivated and optimized for humans looking to understand the logical transformations that the source code has undergone. I generally find merge commits to be contrary to this goal.

In my opinion, merge commits better reflect the history of the code, and thus are also better for humans looking at it to trace back what happened.

But I guess this is neither the time nor the place to discuss that. :)

@BurntSushi
Copy link
Member

Yeah I mean obviously I disagree. I find merge commits bewildering, difficult to follow, messy and encouraging of poor commit hygiene.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants