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: non-alphanumeric characters as separators #3565

Merged
merged 2 commits into from
Dec 12, 2023

Conversation

terriko
Copy link
Contributor

@terriko terriko commented Dec 6, 2023

This switches the logic so we treat all non-alphanumeric characters as separators equivalent to . in version strings. This should make the code more robust to unusual version strings.

Copy link

@antoniogi antoniogi left a comment

Choose a reason for hiding this comment

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

Not a problem with the code itself, but I'm wondering if you need new tests to check that this always does what you want it to do. To be honest, I can't currently think of a case where you could have problems with converting all non-alpha to '.', but, at the same time, something is not feeling right there :| That's why I mention the extra tests.

@terriko
Copy link
Contributor Author

terriko commented Dec 12, 2023

I was going to add the test directly from the version that caused the problem in #3558 but then I realized it wouldn't work because we needed #3566 merged first or the hash would just throw an error. I should be able to update it now.

This switches the logic so we treat all non-alphanumeric characters as
separators equivalent to `.` in version strings.  This should make the
code more robust to unusual version strings.

* Fixes intel#3558 (in that we will be able to handle `~`)

Signed-off-by: Terri Oda <terri@toybox.ca>
@terriko terriko merged commit 9923fe1 into intel:main Dec 12, 2023
21 checks passed
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.

fix: version compare can't handle ~ in version numbers
2 participants