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

Automerge major dependency updates to reduce traffic #11

Merged
merged 2 commits into from Feb 23, 2022
Merged

Conversation

mcollina
Copy link
Member

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

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.
@mcollina
Copy link
Member Author

cc @Fdawgs @Eomm

@Eomm
Copy link
Member

Eomm commented Feb 20, 2022

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 fastify/fastify#3716

@mcollina
Copy link
Member Author

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
Copy link
Member

Eomm commented Feb 20, 2022

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
Copy link
Member Author

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
Copy link
Member

Fdawgs commented Feb 20, 2022

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
Copy link
Member

Eomm commented Feb 20, 2022

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:

  • plugin that does not forward configuration to dependencies: target: major
  • plugin that does not forward configuration to dependencies AND have a low download rate: target: major
  • plugin that forward configuration to dependencies AND have a high download rate target: minor

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

  • high download rate = 10k/week

@mcollina
Copy link
Member Author

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
Copy link
Member

Fdawgs commented Feb 21, 2022

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 #9.

@mcollina
Copy link
Member Author

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

@Fdawgs
Copy link
Member

Fdawgs commented Feb 21, 2022

@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.

@Fdawgs Fdawgs requested a review from Eomm February 22, 2022 16:13
@Eomm Eomm merged commit 8e9fcd2 into main Feb 23, 2022
@Eomm Eomm deleted the mcollina-patch-1 branch February 23, 2022 08:22
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.

None yet

3 participants