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

Only report conflicting dependencies when update is not possible #5237

Merged
merged 1 commit into from
Jun 9, 2022

Conversation

deivid-rodriguez
Copy link
Contributor

@deivid-rodriguez deivid-rodriguez commented Jun 7, 2022

I extracted this commit from #5221, because it's also useful for me to (fix) #4867.

If I understand correctly, both rubygems/rubygems#5520 and #5221 are fixing the same issue but for different ecosystems, and we both run into the same issue where the dependency is properly updated but bin/dry-run.rb still reports conflicts.

@deivid-rodriguez deivid-rodriguez requested a review from a team as a code owner June 7, 2022 10:07
@Nishnha
Copy link
Member

Nishnha commented Jun 8, 2022

Hey @deivid-rodriguez,

Thanks for your work on rubygems/rubygems#5520!

It should make a big impact on the Ruby PRs Dependabot can open! Once it lands I think we will also need to modify the bundler ConflictingDependencyResolver so it no longer flags locked parent dependencies as conflicting.

And yes, your work is similar to what we are doing with npm in #5221--we are leverage Arborist and npm audit fix to identify and update conflicting dependencies.

We had closed out #5221 in favor of #5239 so our PR was more up-to-date. The new PR contains the same commits, and it should be shipped in the next few weeks.

I think we will also need to apply this PR's patch to our internal Updater system for it to work in production--the dry-run.rb script essentially emulates that system for local testing.

Copy link
Contributor

@mctofu mctofu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be fine to pull in early. It's purely cosmetic as the message doesn't prevent the update from happening.

@Nishnha I've already checked our internal updater and it already has this behavior of only reporting conflicts when the update fails.

@Nishnha Nishnha merged commit 3581084 into dependabot:main Jun 9, 2022
@deivid-rodriguez deivid-rodriguez deleted the scope-conflict-reporting branch June 9, 2022 21:34
@mctofu mctofu mentioned this pull request Jun 13, 2022
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.

3 participants