Don't warn for previously constraint dependencies#11091
Conversation
Fixes the case reported at #8870 (comment)
konstin
force-pushed
the
konsti/dont-warn-for-second-req
branch
from
dcb55b5 to
8be831e
Compare
charliermarsh
left a comment
charliermarsh
left a comment
There was a problem hiding this comment.
What if a dependency is listed twice in the same optional group, once with a lower bound and once without?
|
You mean like this? They are already not emitting warnings. (Which looks like a false negative? They are not showing warnings even without the bounds.) |
| locations: &'data IndexLocations, | ||
| workspace: &'data Workspace, | ||
| lower_bound: LowerBound, | ||
| constrained_packages: &'data HashSet<PackageName>, |
There was a problem hiding this comment.
We should be using FxHashSet by default.
| // Support recursive editable inclusions. | ||
| if has_sources | ||
| && !constrained_packages.contains(&requirement.name) | ||
| && requirement.version_or_url.is_none() |
There was a problem hiding this comment.
So, should we remove this? I think it's redundant now? (Also, isn't this wrong? It seems to only look at None, and not the lower bound?)
| bounds: LowerBound, | ||
| ) -> RequirementSource { | ||
| match &requirement.version_or_url { | ||
| None => { |
There was a problem hiding this comment.
I'm actually slightly confused now, hmm... You have this has_lower_bound check in get_constrained_packages, but here we're not looking at the specifier... We're just warning if there's no constraint.
|
Becoming slightly confused about the difference between how |
These are noisy relative to the effect they have on the user. It seems better to prioritize hints on poor resolutions. Notably, it seems hard to make these "not noisy" ref #11091. Does not include the "lowest" resolution mode, in which lower bounds are critical.
2 participants
Fixes the case reported at #8870 (comment)