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

Export CompleteVersion.cmp, Version.rpmvercmp #21

Closed
wants to merge 1 commit into from

Conversation

simon04
Copy link
Contributor

@simon04 simon04 commented May 8, 2018

The drawback of CompleteVersion.Older/Newer/Equal is that it needs to parse the other version (given as string) and returns false if it fails to parse. Thus, afterwards one cannot why one of those tests failed.

This PR allows to Compare two previously parsed CompleteVersions with each other.

@mikkeloscar
Copy link
Owner

@simon04 Thanks for the PR.

I think this is actually exposing a problem in the version API which I think should be fixed by changing the API to take a CompleteVersion for the Older / Newer / Equal methods, rather than exposing the lower level compare API which was intended for internal implementation.

What do you think about that?

@simon04
Copy link
Contributor Author

simon04 commented May 13, 2018 via email

@mikkeloscar
Copy link
Owner

@Morganamilo I think you are one of the biggest consumers of this library, what do you say about breaking the API as discussed here? Would it be a big problem for you?

@Morganamilo
Copy link
Contributor

I don't mind, we mostly just parse srcinfos anyway so little would be effected. There's only one call to .Newer in our codbase as far as I can tell. And even if there were more I don't mind changing stuff.

@mikkeloscar
Copy link
Owner

Replaced by #23

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