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

Use simplified version comparison function #4996

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AltGr
Copy link
Member

@AltGr AltGr commented Jan 14, 2022

As specified in the docs, and not as handled in Debian: we used to run the
Debian comparison from Dose, which handled epoch prefixes and revision
suffixes in an undocumented way (undocumented in opam, not blaming Dose
here!).

Closes #4272

As specified in the docs, and not as handled in Debian: we used to run the
Debian comparison from Dose, which handled epoch prefixes and revision suffixes
in an undocumented way (undocumented in opam, not blaming Dose here!).

Closes ocaml#4272
@kit-ty-kate
Copy link
Member

I thought we had agreed in #4566 to keep it to handle revisions

@AltGr
Copy link
Member Author

AltGr commented Jan 17, 2022

Huh, I am sorry if I skipped that part of a discussion... but I don't see a link between the explicit package revisions discussed in #4566 and the general version-comparison function (on strings) changed here. It may indeed be possible to implement #4566 using debian-like revision syntax, but I don't think it would be a good idea to squeeze that info into the version string instead of using a separate record field.

@rjbou rjbou added this to the 2.2.0~alpha milestone Jan 18, 2022
@AltGr AltGr requested a review from rjbou January 19, 2022 13:42
@kit-ty-kate kit-ty-kate added STATE: NEED INVESTIGATION PR: WIP Not for merge at this stage labels Jan 20, 2022
Copy link
Member

@kit-ty-kate kit-ty-kate left a comment

Choose a reason for hiding this comment

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

I don’t think we should change this now without discussing it extensively. This is really not urgent and this break more things than it tries to solve.

@dra27 dra27 removed this from the 2.2.0~alpha milestone Jul 11, 2022
@kit-ty-kate kit-ty-kate marked this pull request as draft August 13, 2024 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: WIP Not for merge at this stage STATE: NEED INVESTIGATION
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Why is going from version ~x to version ~x~y an upgrade?
4 participants