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

Fix MultiConstraint optimization with version overlap #121

Closed
wants to merge 1 commit into from

Conversation

jderusse
Copy link
Contributor

This is an alternative fix for d07997a#commitcomment-44111784

default:
continue 3;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Not against doing this, but I think some comments would be good here :)

Why !/>/= and !/</= are OK and the rest not?

I see how != is fine to optimize as that's a single gap in the range.

But I don't see why == is ok to optimize, ^1.0 || ^2.0, =2.0.3 means ^1.0 || 2.0.3 but with this optimization it'd become 1.0 - 3.0, 2.0.3 which is only matched by 2.0.3. The ^1.0 is dropped.

I am also not sure why </> are ok in each respective loop. ^1.0 <1.1 || ^2.0 > 2.0.4 getting merged to 1.0 - 3.0, <1.1, > 2.0.4 also seems wrong to me as this suddenly cannot match anything anymore.

So all in all this seems broken, except for != which can be safely kept, but then the question is it worth looping over things just in the odd case we have a !=, which is anyway quite rare..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah.. you're right, I missed to much cases... let's close this

@jderusse jderusse closed this Nov 12, 2020
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.

None yet

3 participants