chdsbd / kodiak

🔮 A bot to automatically update and merge GitHub PRs
https://kodiakhq.com
GNU Affero General Public License v3.0
1.03k stars 65 forks source link

Renovate bot support for automerge by update type #701

Closed erezrokah closed 2 years ago

erezrokah commented 2 years ago

From looking at this regex: https://github.com/chdsbd/kodiak/blob/87cdec3cfe99504b500de1d010b280ec4ca30c81/bot/kodiak/dependencies.py#L6

looks like the following guide will only work for dependabot as renovate bot PR titles are a bit different.

See https://github.com/netlify/cli/pull/3016. The title is fix(deps): update dependency @netlify/build to ^17.1.1 so the version type will not be detected.

Please let me know if I got this wrong or there's another way to handle it (I could configure the renovate PR title, but it feels this should work with the default one).

chdsbd commented 2 years ago

Good idea!

After looking at your example PR, I think we can parse the version numbers from the pull request body to determine the upgrade type

erezrokah commented 2 years ago

Thanks!

I'm adding some more PRs as reference: https://github.com/netlify/cli/pull/3011 https://github.com/netlify/cli/pull/3003 https://github.com/netlify/cli/pull/2998 https://github.com/netlify/cli/pull/2997 https://github.com/netlify/cli/pull/2999

chdsbd commented 2 years ago

@erezrokah Thanks for the links!

It looks like handling dependency updates like https://github.com/netlify/cli/pull/2998 is similar to handling Dependabot updates.

The GitHub Action dependency update (https://github.com/netlify/cli/pull/2997) doesn't follow semver, but I think we can just ignore the v prefix and then treat the update like any other semver.

I was thinking we would consider the lock file update for https://github.com/netlify/cli/pull/2999 a "minor" version upgrade for Kodiak's auto merge purposes.

chdsbd commented 2 years ago

@erezrokah I was looking at the Renovate docs and was wondering where Renovate's auto merge falls short.

It looks like Renovate supports auto merging by update type here: https://docs.renovatebot.com/automerge-configuration/#automerge-non-major-updates

erezrokah commented 2 years ago

Thanks @chdsbd, that's a very good question.

Renovate auto merge falls short when a branch has the Require branches to be up to date before merging protection rule. In that case Renovate's default is to sync PRs when they are 1 commit behind the base branch. See here. That leads to wasteful CI jobs, and can also lead to starvation of non-renovate PRs.

I would like Kodiak to efficiently sync those renovates PRs, and have renovate only update the PR when it's conflicted (by setting rebaseWhen: conflicted.

I thought about opening an issue on Renovate's side, but I think since it's doesn't have the full context of the PRs queue, it has to be done on Kodiak's side.

Please let me know if that makes sense.

chdsbd commented 2 years ago

I think setting rebaseWhen: conflicted might work.

By default, Kodiak will efficiently update the PR right before merging.

chdsbd commented 2 years ago

I now realize we still need to update Kodiak to support auto merging Renovate PRs by update type.

However, if you want to auto merge all Renovate PRs, adding the automerge label will work

erezrokah commented 2 years ago

I now realize we still need to update Kodiak to support auto merging Renovate PRs by update type.

💯

However, if you want to auto merge all Renovate PRs, adding the automerge label will work

Correct, however we can't auto merge Renovate major version updates PRs, as they usually contain breaking changes and require user intervention

chdsbd commented 2 years ago

I've released a new version of Kodiak with support for auto merging Renovate PRs by upgrade type.

I used the PRs you linked as test cases, so those examples you gave should work in the future.

The Kodiak docs on Dependabot should be applicable to Renovate: https://kodiakhq.com/docs/recipes#automated-dependency-updates-with-dependabot-or-renovate

Let me know if you run into any issues.

erezrokah commented 2 years ago

Looks like this is working, see https://github.com/netlify/eslint-config-node/pull/287.

Thanks @chdsbd 🥳

erezrokah commented 2 years ago

Not sure if to open a new issue for it, but we probably want to match https://github.com/netlify/cli/pull/3273 as well. I would consider those as minor versions upgrades.

Happy to learn of another way to put those into the Kodiak's merge queue automatically.

chdsbd commented 2 years ago

@erezrokah I think Kodiak should auto merge that PR as a "patch" update. There's a test using https://github.com/netlify/cli/pull/2999 as an example: https://github.com/chdsbd/kodiak/blob/06a92425ac4869695e4a77e8d815745e2ebd35c6/bot/kodiak/tests/dependencies/renovate_pull_requests/update-patch-lock_file_maintenance.txt

I think that PR isn't merging because Testing for CLI / build (ubuntu-latest, *) (pull_request) is failing

erezrokah commented 2 years ago

Woo, I missed that. Thank you for clarifying ❤️