-
Notifications
You must be signed in to change notification settings - Fork 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
Explain update not possible for bundler #2705
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice find, 👍🏻
7961ad6
to
a0a87fc
Compare
453d73c
to
07e5dac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - comments are all style/naming nits.
bundler/lib/dependabot/bundler/update_checker/parent_dependency_resolver.rb
Outdated
Show resolved
Hide resolved
bundler/spec/dependabot/bundler/update_checker/parent_dependency_resolver_spec.rb
Outdated
Show resolved
Hide resolved
bundler/spec/dependabot/bundler/update_checker/parent_dependency_resolver_spec.rb
Outdated
Show resolved
Hide resolved
bundler/spec/dependabot/bundler/update_checker/parent_dependency_resolver_spec.rb
Outdated
Show resolved
Hide resolved
ba2c145
to
edd78f1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work on this! This looks 💯 🙌
Currently we are actually raising a `can't modify frozen NilClass` error because we attempt to `instance_variable_set` on `nil`, as we try to find `dependency` in the Gemfile. This is a small step towards figuring out what dependency is blocking for an update.
This returns a message from ForceUpdater that explains why a certain update was not possible, by traversing the dependency tree and finding all parent dependencies that are not satisfied by the target version. Few things to improve: - The message is now formatted in the native helper, I would prefer to return raw data from there, and leave any formatting to whatever drives dependabot. - Because we currently depend on the native helper returning an error, it is hard to pass data from the native function to dependabot-core. - We need to call `latest_version_resolvable_with_full_unlock?` in order to hit the error that sets this new message. I'd prefer to be able to just call a method on the UpdateChecker instead. - I dislike having to string compare against the native errors `error_type`, it would be nice to have more granular errors that we can return from there.
Co-authored-by: Pete Wagner <[email protected]>
…endency_resolver_spec.rb Co-authored-by: Pete Wagner <[email protected]>
8cfa8b2
to
2e50b1d
Compare
This adds a new method
blocking_parent_dependencies
toUpdateChecker
, and implements it for bundler. It does this by traversing the dependency tree and finding all parent dependencies that are not satisfied by the target version.It also outputs this in the dry-run script as follows: