search

fastify / workflows

Reusable workflows for use in the Fastify organization
MIT License
11 stars 8 forks source link

Automerge major dependency updates to reduce traffic #11

Closed mcollina closed 2 years ago

mcollina commented 2 years ago

It’s important that we automerge major updates as they are the only ones that will be opened in most of our repos because we do not use lockfiles.

Let’s trust our tests to detect any potential breakage.

Checklist

mcollina commented 2 years ago

cc @Fdawgs @Eomm

Eomm commented 2 years ago

I think this change hurt us more than the benefits it takes: mainly the plugins forward some options to external modules. We had some issues because of that.

Since we are not testing all the external's module options, the CI could be green and merge it. This means that in some cases we should release the plugin as a major version because the options used by the dev through the fastify's plugin could be changed.

In both cases (with or without the automerge), a fastify maintainer needs to check the external dep's release to decide if it should cause a major or a minor release.

A blocked dependabot's PR is easier to manage than reading the merged PRs and going to check them.

For sure and at least, the dependabot configuration should not call these kinds of PRs as chore.

ref https://github.com/fastify/fastify/pull/3716

Eomm commented 2 years ago

Here are some examples:

mcollina commented 2 years ago

We have more than 50 modules in this or, mostly they do not have lock files and they only receive dependency updates with a semver-major. Dealing with a few mistakes is significantly less work than manually checking all dependency updates across the full org.

Let's make it this way: I am the one that have historically done most of the releases across the org.

This changes increases the time it takes to release a module as it requires more manual steps and wait for CI runs.

I will have to further reduce the amount of releases I can do (I already have, partially because of this).

Are others willing to pick up the work? I would be happy to let other take the load. I get upset when others increase my workload without giving me more help.

Eomm commented 2 years ago

Dealing with a few mistakes is significantly less work than manually checking all dependency updates across the full org.

Indeed, I think my approach relies less on the final users (that should open an issue) and moves the burden to the maintainer that publishes the release.

Are others willing to pick up the work?

Yes, I'm. I think to have done it a few times already but I can do more for sure.

In any case, I would like to hear some others @fastify/plugins opinions.

mcollina commented 2 years ago

Indeed, I think my approach relies less on the final users (that should open an issue) and moves the burden to the maintainer that publishes the release.

Exactly. My goal is to push as much work as possible on the users. They are not paying for it, doing QA and opening an issue is the minimum they can do.

Fdawgs commented 2 years ago

Sounds good! I agree, users should be testing updates to plugins in their applications/services using CI themselves. If a dependency breaks a plugin, and it wasn't caught by our own CI, then hopefully users will report this back if it breaks their apps.

Eomm commented 2 years ago

I would propose a parameter instead of removing the target option.

I agree with you at some point, but I would like to define this kind of usage:

Adding the parameter would let us apply this setup in both ways reducing the burden.

Notice that this workflow is used by 2 plugins only, I was planning to spread it during the fastify@v4 upgrade

mcollina commented 2 years ago

How about setting the exclude settings of the automerge action? We can just skip the critical dependencies whose options are exposed to the end users.

Fdawgs commented 2 years ago

Notice that this workflow is used by 2 plugins only, I was planning to spread it during the fastify@v4 upgrade

I have been planning on doing it, was just waiting on a general agreement on script naming in https://github.com/fastify/workflows/issues/9.

mcollina commented 2 years ago

@Fdawgs do you think we could add a workflow input to pass exclude to the automerge action?

Fdawgs commented 2 years ago

@Fdawgs do you think we could add a workflow input to pass exclude to the automerge action?

Sure! Should have time to add it this evening.