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

Mergify: add no-rebase rule #9108

Merged
merged 1 commit into from
Jul 18, 2023
Merged

Mergify: add no-rebase rule #9108

merged 1 commit into from
Jul 18, 2023

Conversation

ulysses4ever
Copy link
Collaborator

If a pull request gets the "merge+no rebase" label, it' will be merged directly to master without a rebase/squash or queue. This can be useful in cases when contributors don't allow updating their branches.

If a pull request gets the "merge+no rebase" label, it' will be merged directly to master without a rebase/squash or queue. This can be useful in cases when contributors don't allow updating their branches.
@ffaf1
Copy link
Collaborator

ffaf1 commented Jul 11, 2023

How frequent is it that contributors disallow changes on their branches? What is their rationale?

@andreabedini
Copy link
Collaborator

I feel mergify should be smarter, the rebase/squash does not necessarilty need to be pushed to the contributor's branch.

@ulysses4ever
Copy link
Collaborator Author

Thanks everyone for feedback!

@ffaf1

How frequent is it that contributors disallow changes on their branches? What is their rationale?

Pretty frequent recently: Oleg (phadej) and Phil (philderbeast) both contribute regularly and don't allow updates for various reasons. I think we ought to support this workflow even if it's rare. Otherwise we may get stuck for no good reason. Of course, the idea is that this workflow is not the one we use by default.

@andreabedini

I feel mergify should be smarter

I mean... I wish it all just worked but it doesn't. Currently, I have to go into back and forth and do manual merges, which I'm not thrilled about. If there are better workflows than what is proposed here, I'd be happy to listen. I was reading Mergify docs more these days, but I don't really see how to fix the issue.

@andreabedini
Copy link
Collaborator

I mean... I wish it all just worked but it doesn't.

Yash, sorry, my comment wasn't very useful ☺️ is #9120 aimed at fixing this? Thank you so much for looking at this!

@ulysses4ever
Copy link
Collaborator Author

No worries! Unfortunately, #9120 fixes another, and much bigger problem where Mergify can't rebase a PR from anyone other than admins (details are here), even if the permission to push to dev branch is given. I don't think there's a workaround for the case when no permission given at all (this case being addressed by this PR). But I should confess that I haven't studied Mergify docs as deep as I wanted to, so maybe there's such workaround...

Copy link
Collaborator

@ffaf1 ffaf1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So be it! No-rebase merging comes at a price for Cabal (a less clean commit history), so hopefully this label will be used only in a sporadic manner.

@ulysses4ever
Copy link
Collaborator Author

@andreabedini in case you missed the discussion o f rebase-less strategy on the IRC, here it is:

f-a
Artem: when you have time, can I ask you to paste a link to a phil PR (one where he does explain why he does not allow rebases). I fear merging will make history for master a tad dirty.

Artem
f-a: yes. But the history is already quite dirty: just look at it. Mergify does create merge commits even with the current workflow. It can probably be improved but needs more reading about Mergify parameters, which I can't afford right now (I'd be happy if someone else could do it...).

More generally, I understand Oleg's concern who doesn't want 3rd party to be able to push to his branch that he can use for some purposes...

Phil said that is because his fork is a part of organization rather than a personal account.

This GH docs page says: “You can only make commits on pull request branches that: ... are on a user-owned fork”. So, most likely, it's impossible for org-owned forks.

@ulysses4ever ulysses4ever mentioned this pull request Jul 14, 2023
2 tasks
@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Jul 16, 2023
@ulysses4ever ulysses4ever merged commit 6af0c08 into master Jul 18, 2023
@ulysses4ever ulysses4ever deleted the ap/mergify-np-rebase-rule branch July 18, 2023 02:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge me Tell Mergify Bot to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants