-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Dependabot bumped a subdependency from 8.x to 9.0 when updating a gem #2246
Comments
This also, presumably, has the side effect of poisoning the compatibility score for this bootstrap version update. Since this autoprefixer-rails update had a 72% pass rate, this bootstrap upgrade had a lower score than it otherwise would have, an 86%. |
Interesting. I think Dependabot / Bundler is doing the right thing here, but it's not totally uncontroversial. Basically, Dependabot is running the equivalent of a normal The conservative part of me wants to change that, and tell Dependabot to only update sub-dependencies if it has to (which is an option in Bundler, although I don't have total confidence in it as I'm not sure how it interacts with the resolver). However, at the moment Dependabot doesn't create any PRs that explicitly target sub-dependencies, so doing so would mean they could really lag behind. Doing the optimistic update when updating a dependency that uses them is an imperfect compromise. In future, I think the right approach for Dependabot might be to have an additional option to create PRs for sub-dependencies (we can actually already do it, and a couple of people have it enabled). If a user enables that then Dependabot should also run updates for top-level dependencies in conservative mode. |
I have the separate subdependency updates enabled which is why I noticed this in the first place :D |
Haha, goes to show how little trouble it's caused on my side that I'd forgotten that! How's it working out for you? |
Quite well, no problems that I've noticed so far. It might cause too many PRs if I had a larger project, but for my use case it's great. |
Also, I think it should be emphasized that this behavior makes the compatibility score less relevant. It means that bootstrap has a patch update with very minimal changes that also bring with it a major version bump for autoprefixer-rails. This causes failures to be attributed to bootstrap when they're actually problems with autoprefixer-rails. I don't know what the correct solution is here, but I think that's the most important problem this causes. That, and maybe the fact that it would cause major dependency updates to happen under the user's nose. |
At the least, I think the PR should mention that autoprefixer-rails was also updated. For example, the PR title could be changed from (also, apologies for the number of comments! I keep thinking of things I want to add 😄) |
I wish Dependabot will run Thanks! |
While I think we should allow semver to do its job, because dependabot doesn't include the changelog of the sub-dependencies, I actually have to go reach out and see if the sub-dependencies have any sneaky breaking changes. All it would take is some configuration option for |
I don't mind if subdependencies are updated, but this should be made explicit in the PR, ideally with listing the relevant changelog etc. |
Thanks for the feedback. As you can imagine, we're a little snowed under with requests right now! The reason Dependabot doesn't use On changelogs for sub-dependencies, we don't currently pass that information out of dependabot-core, so it's a non-trivial change. I agree it would be an improvement, though. |
@greysteil Any progress on making |
Dependabot updated bootstrap from 4.1.1 to 4.1.2, but in the process also updated autoprefixer-rails from 8.6.5 to 9.0.0.
connorshea/mdn-compat-data-explorer#117
I think this just might be a limitation of bundler (IIRC it updates the subdependencies of any gems that are updated, not sure if there's a flag you can pass to stop this?). But I feel like this isn't the ideal behavior, it should at least block the bootstrap PR from being merged since a dependency had a major update.
The text was updated successfully, but these errors were encountered: