Skip to content

Use generic pubgrub incompatibility reason #3335

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

Merged
merged 6 commits into from
May 8, 2024

Conversation

konstin
Copy link
Member

@konstin konstin commented May 2, 2024

Pubgrub got a new feature where all unavailability is a custom, instead of the reasonless UnavailableDependencies and our custom String type previously (pubgrub-rs/pubgrub#208). This PR introduces a UnavailableReason that tracks either an entire version being unusable, or a specific version. The error messages now also track this difference properly.

The pubgrub commit is our main rebased onto the merged pubgrub-rs/pubgrub#208, i'll push konsti/main-rebase-generic-reason to main after checking for rebase problems.

@konstin konstin added the error messages Messaging when something goes wrong label May 2, 2024
@konstin konstin requested a review from zanieb May 2, 2024 08:13
Copy link

codspeed-hq bot commented May 2, 2024

CodSpeed Performance Report

Merging #3335 will not alter performance

Comparing konsti/pubgrub-generic-incompatibility (b491904) with main (bd7860d)

Summary

✅ 12 untouched benchmarks

@konstin konstin force-pushed the konsti/pubgrub-generic-incompatibility branch 2 times, most recently from a64cfed to 0b93a71 Compare May 3, 2024 13:12
@konstin
Copy link
Member Author

konstin commented May 3, 2024

The whole plural handling imho makes a lot of effort over just reformulating plural-independent

@konstin konstin force-pushed the konsti/pubgrub-generic-incompatibility branch from 6f1e47d to a56a2d4 Compare May 7, 2024 11:02
╰─▶ Because black==23.10.1 was not found in the cache and you require black==23.10.1, we can conclude that the requirements are unsatisfiable.
╰─▶ Because black was not found in the cache and you require black==23.10.1, we can conclude that the requirements are unsatisfiable.
Copy link
Member Author

Choose a reason for hiding this comment

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

The message improves here because it's the versions of black, not that specific version, that is missing in the cache

@konstin konstin marked this pull request as ready for review May 7, 2024 11:05
@zanieb zanieb self-requested a review May 7, 2024 13:14
╰─▶ Because example-a-961b4c22==1.0.0 was not found in the package registry and you require example-a-961b4c22==1.0.0, we can conclude that the requirements are unsatisfiable.
╰─▶ Because example-a-961b4c22 was not found in the package registry and you require example-a-961b4c22==1.0.0, we can conclude that the requirements are unsatisfiable.
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused why does this test error? Shouldn't this resolve cc @charliermarsh

Copy link
Member

@zanieb zanieb left a comment

Choose a reason for hiding this comment

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

Nice!

@konstin konstin enabled auto-merge (squash) May 8, 2024 08:37
@konstin konstin merged commit 1ad6aa8 into main May 8, 2024
@konstin konstin deleted the konsti/pubgrub-generic-incompatibility branch May 8, 2024 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error messages Messaging when something goes wrong
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants