Skip to content

Implement the explicit automergeStrategy for the GitHub platform. #10869

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

Closed
jbirch-atlassian opened this issue Jul 15, 2021 · 12 comments · Fixed by #34537
Closed

Implement the explicit automergeStrategy for the GitHub platform. #10869

jbirch-atlassian opened this issue Jul 15, 2021 · 12 comments · Fixed by #34537
Assignees
Labels
core:automerge Relating to Renovate's automerge capabilities platform:github GitHub Platform priority-2-high Bugs impacting wide number of users or very important features type:feature Feature (new functionality)

Comments

@jbirch-atlassian
Copy link
Contributor

What would you like Renovate to be able to do?

The automergeStrategy configuration item introduced in #10627 should be implemented for the GitHub platform, allowing users to configure the explicit merge strategy used by Renovate when automerging pull requests

Did you already have any implementation ideas?

The optional MergeStrategy argument can be provided to the GitHub mergePr implementation.The current implementation uses the merge strategy configured on the repository at the time it is internally constructed, with preferences based on which features are allowed. The Renovate-specific strings for automergeStrategy can be mapped to GitHub-specific strings when submitting the merge request to GitHub or aborted if the configured strategy is not listed as available.

If #10830 has been implemented, the arguments to mergePr may be different than what has been listed here. We should also remember to clean up the missing feature notice in lib/platform/github/index.md and update the description in docs/usage/configuration-options.md if necessary.

@jbirch-atlassian jbirch-atlassian added priority-5-triage status:requirements Full requirements are not yet known, so implementation should not be started type:feature Feature (new functionality) labels Jul 15, 2021
@HonkingGoose HonkingGoose added core:automerge Relating to Renovate's automerge capabilities platform:github GitHub Platform labels Jul 16, 2021
@rarkins rarkins added priority-4-low Low priority, unlikely to be done unless it becomes important to more people status:ready and removed priority-5-triage status:requirements Full requirements are not yet known, so implementation should not be started labels Nov 25, 2022
@HonkingGoose
Copy link
Collaborator

Do we still want to work on this issue? Renovate can now put its PRs in GitHub's Merge Queue. Does that change anything for us?

In any case, we still mention the automergeStrategy in our "GitHub Platform" docs: 1

Features awaiting implementation

The automergeStrategy configuration option has not been implemented for this platform, and all values behave as if the value auto was used. Renovate will use the merge strategy configured in the GitHub repository itself, and this cannot be overridden yet

The general automergeStrategy 2 config option docs do not mention that on GitHub Renovate always uses automergeStrategy=auto though. Should I add a admonition to the config option docs to highlight this. I guess most of our users are on GitHub?

Footnotes

  1. https://docs.renovatebot.com/modules/platform/github/#features-awaiting-implementation

  2. https://docs.renovatebot.com/configuration-options/#automergestrategy

@rarkins
Copy link
Collaborator

rarkins commented Apr 4, 2023

It's of less value if merge queue becomes common/standard.

From https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-a-merge-queue#managing-a-merge-queue it indicates that a merge queue has one way to merge, so then we don't have to choose, we just queue.

The general automergeStrategy 2 config option docs do not mention that on GitHub Renovate always uses automergeStrategy=auto though. Should I add a admonition to the config option docs to highlight this. I guess most of our users are on GitHub?

Isn't the default value auto, meaning the GitHub App just uses default? I'm not sure why to mention anything special, unless I'm missing something.

@viceice
Copy link
Member

viceice commented Apr 4, 2023

i think this is still valuable, as not all users will use the new merge queue feature but have multiple merge strategies enabled

@HonkingGoose
Copy link
Collaborator

HonkingGoose commented Apr 4, 2023

Isn't the default value auto, meaning the GitHub App just uses default? I'm not sure why to mention anything special, unless I'm missing something.

It looks like Renovate ignores the automergeStrategy option on the GitHub platform, and behaves like the user set automergeStrategy=auto. At least that's what I'm getting from this quote from our docs:

The automergeStrategy configuration option has not been implemented for this platform, and all values behave as if the value auto was used. Renovate will use the merge strategy configured in the GitHub repository itself, and this cannot be overridden yet

If that quote is correct (check the code to see the actual behavior maybe?), then I probably should add an admonition to the automergeStrategy config option docs. That admonition will probably reach more users than the Features awaiting implementation notice on the GitHub Platform docs.

@viceice
Copy link
Member

viceice commented Apr 4, 2023

maybe just take some time to implement this feature. should be doable in a couple of hours 😜

@jbirch-atlassian
Copy link
Contributor Author

jbirch-atlassian commented Apr 5, 2023

Hey there, I'm the original author of that line. Apologies for any confusion it causes — I know a lot of my other comments have been rewritten to use more approachable language, and this might just be another one of them.

The confusion is coming from what auto means. Conceptually, auto means "Let the platform do whatever it wants to when I say 'merge' without any other instructions". Practically, this doesn't mean "ignore what was set and thread through a value of auto", this means "the GitHub Platform code doesn't care what the value is at all".

"Unset" and "Set to auto" are functionally equivalent everywhere. But in the GitHub Platform case, "any value" and "Set to auto" are equivalent, because it just never reads the value.

@Fab1n
Copy link

Fab1n commented Oct 17, 2023

Reading through all of this I am not sure if I understand the current state.

Setting automergeStrategy to squash for example doesn't seem to do anything, is that correct? I still see PRs being merged without squasing them.

@rarkins
Copy link
Collaborator

rarkins commented Oct 17, 2023

The "current state" is that this issue is open, which means that feature is not yet implemented for github.

Renovate attempts to discover which merge methods are allowed for the repo:

if (repo.rebaseMergeAllowed) {
config.mergeMethod = 'rebase';
} else if (repo.squashMergeAllowed) {
config.mergeMethod = 'squash';
} else if (repo.mergeCommitAllowed) {
config.mergeMethod = 'merge';
} else {
// This happens if we don't have Administrator read access, it is not a critical error
logger.debug('Could not find allowed merge methods for repo');
}

GitHub lets you control which merge types are allowed, if you configure that then naturally Renovate won't be able to bypass those and will follow whatever is configured.

I also recommend using GitHub-native automerge

@Fab1n
Copy link

Fab1n commented Oct 19, 2023

@rarkins thank you very much for your quick summary.
Sadly we have a repo where we have all 3 merge strategies allowed lately and thus we need the strategy selection to override the standard one (which is merge) to be able to use 'squash'.

Until then no automerge for us.

@rarkins rarkins added priority-2-high Bugs impacting wide number of users or very important features and removed priority-4-low Low priority, unlikely to be done unless it becomes important to more people labels Feb 27, 2025
@RahulGautamSingh
Copy link
Collaborator

If automergeStrategy configured by the user is not valid i.e. fast-foward which is not supported by Github.

  1. Should we abort the run as suggested in the issue description Or,

  2. Fallback to the existing behaviour

Current Behaviour:

  1. Check mergeStrategy set for the repository and use it
  2. if that is absent then try all the merge strategies to see which works

@rarkins
Copy link
Collaborator

rarkins commented Feb 27, 2025

Let's log a warning if it's invalid and treat that as "no strategy" if so

@renovate-release
Copy link
Collaborator

🎉 This issue has been resolved in version 39.185.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 31, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
core:automerge Relating to Renovate's automerge capabilities platform:github GitHub Platform priority-2-high Bugs impacting wide number of users or very important features type:feature Feature (new functionality)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants