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

Github's conflict resolution creates merge conflicts that are then flagged by rustbot #136521

Open
safinaskar opened this issue Feb 4, 2025 · 4 comments
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.

Comments

@safinaskar
Copy link
Contributor

This repo is probably misconfigured: merge commits probably are allowed in the repo's settings.

Recently I submitted this PR: #136471 . Github shows me so-called "new PR merge experience" ( https://github.com/orgs/community/discussions/143787 ). At some point Github detected a conflict between my branch and mainline and showed me a message about this. Github interface offered me to fix this conflict directly in Github interface. I agreed and fixed (in Github web interface). But then immediately @rustbot said me that merges are not allowed. So I have to remove my merge commit and do rebase instead on my computer locally.

So, for unknown reasons Github offers a user to create merge commit, despite they are not allowed. This is probably repo misconfiguration. If there is no misconfiguration, then this is a bug in Github's "new PR merge experience" and should be reported to https://github.com/orgs/community/discussions/143787 .

I consider this a high-priority bug, because it presents a major contribution roadblock. This will definitely scary away potential contributors. Think about this: at first you are asked to do a merge, but then immediately @rustbot says to you that you did something wrong and should undo. I think many potential contributor will simply think: "I don't want to contribute at all".

@rustbot label T-infra A-contributor-roadblock P-high

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Feb 4, 2025
@rustbot
Copy link
Collaborator

rustbot commented Feb 4, 2025

Error: Label P-high can only be set by Rust team members

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

@safinaskar
Copy link
Contributor Author

@rustbot label T-infra A-contributor-roadblock

@rustbot rustbot added A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Feb 4, 2025
@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Feb 4, 2025
@RalfJung
Copy link
Member

RalfJung commented Feb 4, 2025

Note that we do have some PRs where merge commits are allowed and even required, such as subtree syncs. So any repo setting that forbids merge commits entirely does not work for us. So I don't think there is a misconfiguration here.

So if Github only offers "forbid merge commits entirely" and "direct users to resolve conflicts by adding merge commits", then neither option works for us and this should be reported with Github. (I have no idea if the "new merge experience" changes anything here.)

@RalfJung RalfJung changed the title This repo is probably misconfigured: merge commits probably are allowed in the repo's settings Github's conflict resolution creates merge conflicts that are then flagged by rustbot Feb 4, 2025
@rustbot
Copy link
Collaborator

rustbot commented Feb 4, 2025

Error: Label P-high can only be set by Rust team members

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants