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

Feat/version compare #56

Merged
merged 1 commit into from
May 27, 2020
Merged

Conversation

amelhusic
Copy link
Collaborator

No description provided.

@gdubicki
Copy link
Contributor

gdubicki commented May 21, 2020

While we are here shouldn't we rather start checking for version X.*, not >= X.0 ?
(<- updated to be more clear)

Not only theoretically major semver version uptick may cause issues but as issue #6 shows it also happens in practice.

@amelhusic
Copy link
Collaborator Author

Hi @gdubicki. Not sure what do you mean by X.*?
HAProxy semver versions are in either major.minor or major.minor.patch format.
Dataplaneapi follows major.minor.patch format.

Here we check minimum required versions.

@gdubicki
Copy link
Contributor

I mean that I think that we should test if the major version is equal to expected, not equal or greater than.

@amelhusic
Copy link
Collaborator Author

Got it. @ShimmerGlass what do you think?

@amelhusic amelhusic force-pushed the feat/version-compare branch from 5b057bd to 5caf2fa Compare May 26, 2020 12:10
@aiharos aiharos requested a review from ShimmerGlass May 26, 2020 12:16
@ShimmerGlass
Copy link
Collaborator

I agree with @gdubicki, it's probably better to require a specific major version

@amelhusic amelhusic force-pushed the feat/version-compare branch from 5caf2fa to 03a8805 Compare May 26, 2020 12:29
@amelhusic
Copy link
Collaborator Author

It makes sense to me too. Updated.

…urrent program version

Fixed regex used to extract program version.
`version` arg was unused. Now, it is renamed as `minVer` and used in comparison.
Additional `compareVersion` func added to compare minVer with currVer.
Comparision with `strings.Compare` didn't worked for cases like `0.1.10 > 0.1.2`
and therefore it is removed.
Added unit test for `compareVersion` func.
@amelhusic amelhusic force-pushed the feat/version-compare branch from 03a8805 to 9d0c478 Compare May 27, 2020 07:28
@aiharos aiharos merged commit 9b0cea0 into haproxytech:master May 27, 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.

4 participants