Remove check_merge_commits test
#12829
Conversation
This adds an `in-rust-tree` feature that will be enabled when rust-analyzer is built from `rust-lang/rust`. Due to the way "git subtree" works, that test _will_ find merge commits and fail, so we simply skip it.
Rationale: Merge commits will probably end up in `rust-lang/rust-analyzer` when doing "rust=>ra" syncs anyway. It could be changed to only check for merge commits in non-sync PRs, but it's "probably not worth the hassle"
Since it's unused for now -it'll be re-introduced along with the upcoming `proc-macro-srv/sysroot` feature.
fasterthanlime
changed the title
check_merge_commits test
|
@bors r+ |
|
☀️ Test successful - checks-actions |
|
Could you elaborate on why this was done? The reason we want to avoid merge commits not authored by bors is that they make the history hard to read since they are mostly just noise that a proper rebase avoids). Newcomers often create many merge commits when updating their PRs (since GitHub strongly encourages it and it's also git's default behavior), so avoiding merge commits there is now responsibility of the maintainers instead of an automated check. This PR does not acknowledge that and claims that there is no maintainer impact. Do we not want to avoid merge commits anymore? Is there anything that could be done to detect and mark merge commits coming from git-subtree? I am not clear about why using subtrees means that we have to deal with merge commits. |
I forgot who "signed off" on this change, and whether it happened on Zulip or GitHub. I'm aware of the reasons to want to avoid merge commits, and I suppose there might be a way to bring the check back in some form. I would have to dive into this again, but off the top of my head: I feel like the merge commits created by syncs might not be authored by bors (but by whoever is opening the sync PR), which seems like it would complicate the check somewhat.
Anyone who's interested in bringing back that check can look at https://github.com/rust-lang/rust-analyzer/pull/12871/commits for an example of a sync. |
|
Ye the merge commit seems to come from the author, that seems unfortunate... |
Due to the way "git subtree" works, the
check_merge_commitstest will find merge commits and fail, so we simply skip it.This changed is tracked in:
Maintainer impact: none