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

Use branch automerge strategy #1

Merged
merged 1 commit into from
Oct 28, 2023
Merged

Use branch automerge strategy #1

merged 1 commit into from
Oct 28, 2023

Conversation

pat-s
Copy link
Contributor

@pat-s pat-s commented Oct 27, 2023

Strategy description:

  • Renovate first creates a branch and no PR
  • If tests pass, Renovate pushes a commit directly to the base branch without PR
  • If tests fail, Renovate raises a PR for you to review

Major updates are never automerged.

Source: https://docs.renovatebot.com/noise-reduction/#branch-automerging

Copy link
Contributor

@qwerty287 qwerty287 left a comment

Choose a reason for hiding this comment

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

Major go dependency updates shouldn't be automerged if the import paths were changed (e.g. github.com/x/y/v1 -> github.com/x/y/v2`) because renovate can't handle this.

Unfortunately the ci checks pass in this case. Can we prevent this?

@pat-s
Copy link
Contributor Author

pat-s commented Oct 28, 2023

Major deps are should never get automerged as we have "automergeMajor": false, set.

Can you show a PR where this happened? I only found the "non-major golang deps" ones which look good to me (e.g. woodpecker-ci/woodpecker#2566).

@qwerty287
Copy link
Contributor

See e.g. woodpecker-ci/woodpecker#2573

Check what renovate did (first commit) and what I had to do to fix this manually (second commit)

@pat-s
Copy link
Contributor Author

pat-s commented Oct 28, 2023

This is a major update and wouldn't have been automerged.

Not all updates work OOB and this is more often to happen for major ones anyhow. Not sure if the manual work done by you is a one time fix or would be required next time again (for this I am not enough of an expert on go). I also can't see if the CI of the first commit succeeded (it's yellow and I'm on the phone).

But WRT to your initial comment about potential automerges which don't work: I think for golang, as long as the tests work it should be fine? I.e. due to the coverage, CI should always fail if there really is an issue - shouldn't it?

@qwerty287
Copy link
Contributor

But WRT to your initial comment about potential automerges which don't work: I think for golang, as long as the tests work it should be fine? I.e. due to the coverage, CI should always fail if there really is an issue - shouldn't it?

Not if there's a change to import path after a major upgrade (which is excluded anyways, so my issue no longer applies).

In these cases, renovate just appends the new dependency path to go.mod but does not do anything else. This is useless and does not actually update it. Renovate opens a new PR after this which does update the original dep and there it also fails.

@pat-s
Copy link
Contributor Author

pat-s commented Oct 28, 2023

If this is a specific issue with major upgrades, I guess something can/should be done on the renovate side then.
Maybe somebody already reported it?

The only thing I am concerned about is if something would be merged which causes an issue/does not work. Imperfect updates of certain deps are not great but something we/you can easily workaround.

But thanks for asking and for the approval! Let's see how it goes, I have great hopes that it will help in a good way.

@pat-s pat-s merged commit ade8293 into main Oct 28, 2023
@pat-s pat-s deleted the branch-automerge branch October 28, 2023 13:39
@pat-s
Copy link
Contributor Author

pat-s commented Oct 28, 2023

@anbraten could you activate the renovate-approve bot for all repos in the org?

@qwerty287
Copy link
Contributor

If this is a specific issue with major upgrades, I guess something can/should be done on the renovate side then.
Maybe somebody already reported it?

Yes: renovatebot/renovate#21010

@anbraten
Copy link
Member

@anbraten could you activate the renovate-approve bot for all repos in the org?

How do I have to do this again 😅

@anbraten
Copy link
Member

Should be adjusted now

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

Successfully merging this pull request may close these issues.

3 participants