notes icon indicating copy to clipboard operation
notes copied to clipboard

Automatically merging dependency PRs

Open varl opened this issue 5 years ago • 12 comments

Outcome of FEDB: #70

Trial

Allow Dependabot to merge a sub-set of dependencies to master in a low-risk repository: cli-style

Background and discussion

The up-to-date automerged dependencies can be seen in cli-style/.dependabot/config.yml at any time. At the time of writing the strategy is to automatically merge any minor updates to any @dhis2 scoped package and merge all patch versions for known security fixes. Others will be manually handled.

Going forward

Will run the trial for 1 week (2019-10-14 to 2019-10-18) in cli-style before rolling out to ui-widgets.

If everything keeps going all right we will identify another candidate from the Dependabot enabled repos.

varl avatar Oct 14 '19 07:10 varl

Rolling out to ui-widgets: https://github.com/dhis2/ui-widgets/commit/2d66eaf386a288ae2c1e34ea8850ea6f11547e74

varl avatar Oct 18 '19 05:10 varl

I've disabled the required reviews check for dhis2/ui-widgets and dhis2/cli-style to allow Dependabot to automatically merge some PRs.

We should still wait until we have approving reviews before merging, so some discipline is required.

I haven't figured out a way to allow Dependabot to by-pass the "review required" check, even if Dependabot has special access we get this message: https://github.com/dhis2/ui-widgets/pull/264#issuecomment-572414887

varl avatar Jan 10 '20 08:01 varl

I have been monitoring the Dependabot activity and have it as part of my weekly routine to tackle dependency updates to ensure that we keep things up-to-date.

Either this needs to be part of everyones routine, or we try to automate more of it so we can focus on the cases user intervention is required.

varl avatar Jan 10 '20 08:01 varl

Either this needs to be part of everyones routine, or we try to automate more of it so we can focus on the cases user intervention is required.

Should we first have more extensive tests on the repos where we enable automatic dep updates? Or do we just fix it on master if it breaks anything?

I'll also make it part of my routine to check the dependabot upgrades 👍

ismay avatar Jan 13 '20 08:01 ismay

Ideally we'd have good enough checks that we ultimately trust them before auto-merging.

My experience (so far) based on validating and merging hundreds of these dependency upgrades over the course of a year is that when quickly merging patch and minor releases they rarely pose a problem. I've only hit two instances that have accidentally published a breaking change in a minor or fix version (and it was the same package at two different points in time).

The real problems start happening when the dependencies have gotten stale and we have to jump ahead 10 releases to catch up. E.g. if we look at the app-hub repo, there are dependency updates that jump ahead a lot of releases. No one is confident in merging these, so they just slush around.

In contrast, look at ui-core. We constantly and aggressively merge in dependency updates, and even before we had e2e tests, we have only seen a dependency update break the lib once. In that case it was our mistake though, it was a major version bump with a breaking change we forgot to accommodate for.

For now, I've only enabled auto-merging @dhis2-scoped packages for minor and patch versions, and auto-merging security fixes:

  • https://github.com/dhis2/cli-style/blob/master/.dependabot/config.yml#L9-L16
  • https://github.com/dhis2/ui-widgets/blob/master/.dependabot/config.yml#L9-L16

Everything else still has to be merged manually, and we can gradually roll out the functionality as we add more tests.

varl avatar Jan 13 '20 09:01 varl

All right, sounds good!

ismay avatar Jan 13 '20 10:01 ismay

Update

  • App Hub now automatically merges minor and patch updates to all dependencies. It merges into next which is the staging branch for testing before production. So far it seems to be working well.

  • d2-style has gotten the same treatment as well.

Future

I think that we are in a position where we can start rolling out automatic merges of dependency updates to additional repositories. Once we've done it for the libs (ui, cli, analytics) the next phase is rolling it out to the apps.

varl avatar Mar 17 '20 13:03 varl

Combined with the functionality semantic-release gives us to publish to dist-tags (covered in #72), we can automatically merge pretty much all minor and patch updates and use the flow of merging from next to master to manually trigger a release, and therefor allow chore updates to be released when we need them to.

This would e.g. make translations go out faster as well as security updates.

varl avatar Apr 24 '20 09:04 varl

When Dependabot was merged into GitHub, they dropped support for automerging, so this needs to be addressed elsewhere.

Enter Kodiak.

Currently trying out the automerge functionality in these repos:

  • https://github.com/dhis2/prop-types
  • https://github.com/dhis2/cli-style
  • https://github.com/dhis2/app-hub (on next)

To set it up:

  • Organization: Grant the Kodiak GitHub App access to the repo
  • Repo: In the branch protection settings, we need to add "kodiakhq" to the list of users that can push to restricted branches.
  • Repo: Enable "Require pull request reviews before merging", unless you want Kodiak to truly automerge.
  • Repo: Add the .github/.kodiak.toml file
  • Repo: Update the .github/dependabot.yml config to add the automerge label

The configuration files will eventually be built into cli-style as well.

Bonus: If you add the label automerge to any PR, then Kodiak will merge it when all the checks pass.

varl avatar Oct 26 '20 13:10 varl

Ah interesting.

ismay avatar Nov 02 '20 11:11 ismay

Quoting a potential solution to this from slack: https://dhis2.slack.com/archives/CJF3MDK8Q/p1628155318006300

In the past we've talked about how to improve our dependency updates, i.e. how to make the process simpler so that the dependencies update PRs don't pile up. I've tried out https://www.whitesourcesoftware.com/free-developer-tools/renovate/ for a bit, and I think it would be an improvement over dependabot. I think it could help us cut down on the hours needed for updates. Things I like about it:

  • It's aware of updates that come from monorepos, and bundles all monorepo updates together (dependabot has an open issue for this that has been open for several years, no indication it's being worked on)
  • It has the same rights as a user when running CI tasks, so it can run all tests, which means we can merge with a little more certainty (and less workflow checks are required)
  • It's aware of library vs apps and handles updates to them appropriately (so it pins deps for apps for example, and uses a range for library deps)

All in all it's a lot more configurable than dependabot in my experience and I found it a lot nicer to work with. I think that for our apps that have decent test suites it seems safe to move them to automerging minor and patch updates with renovate.

Docs are here: https://docs.renovatebot.com/configuration-options. Check them out, there are a ton of useful options imo.

ismay avatar Aug 09 '21 09:08 ismay

Currently trying out Renovate in cli-style.

varl avatar Sep 28 '21 09:09 varl