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

Replace versioncheck.Version with semver.ParseTolerant #1561

Merged
merged 1 commit into from
May 3, 2023

Conversation

michi-covalent
Copy link
Contributor

@michi-covalent michi-covalent commented May 1, 2023

ParseCiliumVersion uses versioncheck.Version from Cilium's versioncheck
package (1), which ignores certain pre-release identifiers. Silently
ignoring certain pre-release identifiers can be confusing since you
might end up installing a different version of Cilium than what you
specified.

This commit replaces versioncheck.Version with semver.ParseTolerant (2)
in ParseCiliumVersion function so that cilium-cli interprets the version
provided via the --version flag as is.

Signed-off-by: Michi Mutsuzaki michi@isovalent.com

@michi-covalent michi-covalent temporarily deployed to ci May 1, 2023 23:49 — with GitHub Actions Inactive
@michi-covalent michi-covalent force-pushed the pr/michi/remove-parse-version branch from c9b81ed to d4fa6a9 Compare May 2, 2023 00:31
@michi-covalent michi-covalent temporarily deployed to ci May 2, 2023 00:31 — with GitHub Actions Inactive
@michi-covalent michi-covalent marked this pull request as ready for review May 2, 2023 01:20
@michi-covalent michi-covalent requested review from a team as code owners May 2, 2023 01:20
Copy link
Contributor

@chancez chancez left a comment

Choose a reason for hiding this comment

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

A few questions about the approach being taken.

ParseCiliumVersion uses versioncheck.Version from Cilium's versioncheck
package ([1]), which ignores certain pre-release identifiers. Silently
ignoring certain pre-release identifiers can be confusing since you
might end up installing a different version of Cilium than what you
specified.

This commit replaces versioncheck.Version with semver.ParseTolerant ([2])
in ParseCiliumVersion function so that cilium-cli interprets the version
provided via the --version flag as is.

[1]: https://pkg.go.dev/github.com/cilium/cilium/pkg/versioncheck#Version
[2]: https://pkg.go.dev/github.com/blang/semver#ParseTolerant

Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
@michi-covalent michi-covalent force-pushed the pr/michi/remove-parse-version branch from d4fa6a9 to 74d64d9 Compare May 2, 2023 21:18
@michi-covalent michi-covalent temporarily deployed to ci May 2, 2023 21:18 — with GitHub Actions Inactive
@michi-covalent michi-covalent changed the title Replace ParseCiliumVersion with semver.ParseTolerant Replace versioncheck.Version with semver.ParseTolerant May 2, 2023
@tklauser tklauser merged commit b5fff0b into main May 3, 2023
14 checks passed
@tklauser tklauser deleted the pr/michi/remove-parse-version branch May 3, 2023 08:46
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

6 participants