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

chore: Enable renovate automerge for non-major updates #5182

Merged
merged 3 commits into from
Apr 21, 2023

Conversation

MartinWitt
Copy link
Collaborator

@MartinWitt MartinWitt commented Apr 19, 2023

After discussion with @monperrus this renovate config enables automerge of green PR updates from renovate. I have changed the master protection rules to let renovate bypass them. I removed some old mockito rule which seems useless.

renovate.json Outdated
"patch",
"pin",
"digest",
"fix"
Copy link
Contributor

Choose a reason for hiding this comment

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

fix is not an allowed value based on the documentation over here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed it

@slarse
Copy link
Collaborator

slarse commented Apr 19, 2023

Just for clarity, could we include in the commit message on master that this does not apply to major version updates? Something like "Enable renovate automerge for non-major updates".

@MartinWitt
Copy link
Collaborator Author

Just for clarity, could we include in the commit message on master that this does not apply to major version updates? Something like "Enable renovate automerge for non-major updates".

Yes I can do this, but this is also an open point for discussion. Shall we automerge major version changes?

@slarse
Copy link
Collaborator

slarse commented Apr 19, 2023

Just for clarity, could we include in the commit message on master that this does not apply to major version updates? Something like "Enable renovate automerge for non-major updates".

Yes I can do this, but this is also an open point for discussion. Shall we automerge major version changes?

I would spontaneously say no. Even if our tests pass, there may be problems for consumers when major library versions change. I think a major version upgrade always warrants at least a cursory inspection of the consequences of adopting it.

@algomaster99
Copy link
Contributor

algomaster99 commented Apr 19, 2023

warrants at least a cursory inspection of the consequences of adopting it

It is hard to narrow down which part of the code would be affected by the dependency update, especially on such a big codebase. So I am not sure where I would look for possible failures.

For example, Spoon relies heavily on eclipse JDT so if there is an incompatible change, our tests would most likely detect it. But for other less used dependencies, our tests may not detect it, but I imagine it would be rare.

@MartinWitt MartinWitt changed the title chore: Enable renovate automerge chore: Enable renovate automerge for non-major updates Apr 19, 2023
@MartinWitt
Copy link
Collaborator Author

Okay, let's start without major versions and see how it goes? We can change this any moment if we believe merging majors is useful too.

renovate.json Outdated Show resolved Hide resolved
Copy link
Collaborator

@slarse slarse left a comment

Choose a reason for hiding this comment

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

Let's go? Any objections @SirYwell @I-Al-Istannen @algomaster99 ?

@MartinWitt MartinWitt changed the title chore: Enable renovate automerge for non-major updates review: chore: Enable renovate automerge for non-major updates Apr 21, 2023
@monperrus monperrus changed the title review: chore: Enable renovate automerge for non-major updates chore: Enable renovate automerge for non-major updates Apr 21, 2023
@monperrus monperrus merged commit 189025e into master Apr 21, 2023
@monperrus monperrus deleted the MartinWitt-patch-3 branch April 21, 2023 12:28
@monperrus
Copy link
Collaborator

Really nice, thanks a lot @MartinWitt and all for your inputs

@MartinWitt
Copy link
Collaborator Author

Currently, renovate is not doing any auto-merge. I believe a config change only affects new pull requests. In the pull requests, the lights for the auto-merge are green, so our config is correct. Lets merge them and see if it works for new pull-requests?

@slarse
Copy link
Collaborator

slarse commented Apr 26, 2023

@MartinWitt It feels like something is wrong, because if you have a look at the recent PRs they say "Automerge: Enabled". Maybe some permission is missing for the bot s.t. it cannot automerge? Perhaps the branch protection bypass isn't working as intended?

@slarse
Copy link
Collaborator

slarse commented Apr 26, 2023

Don't we have "Pull request required" enabled? I'm not boss enough to check the branch protection rules but seems like that would be a show-stopper (for some reason??).

https://docs.renovatebot.com/key-concepts/automerge/#pull-requests-required

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.

6 participants