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

Revert "wolfictl update: transform-version needs to run before versio… #415

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

Conversation

rawlingsj
Copy link
Member

…ns are sorted and compared"

This reverts commit bb20ed5.

There's more edge cases to consider which will need more test cases. We've seen an unexpected version update PR come in that uses the version transforms wolfi-dev/os#6255. Let's roll this commit back and add more tests to cover more edge cases.

…ns are sorted and compared"

This reverts commit bb20ed5.
@rawlingsj rawlingsj marked this pull request as draft October 4, 2023 19:53
@rawlingsj
Copy link
Member Author

Sigh - need to think about this a bit more and look at more test cases. The original implemenation of version transforms means we were missing some updates

wolfictl check update  --override-version 0.0.1 openjdk-21

original 21.9 is wrong it should be [jdk-21-ga](https://github.com/openjdk/jdk21u/releases/tag/jdk-21-ga) however I think that's partly down to an issue with the regex. If you ignore the regex issue preventing-ga then the fix my change made would work, as it would get the next most recent release jdk-21+35.

i.e. the code in wolfictl was incorrectly resolving 21.9 which it should be 21.35

So that seemed to be working but we've had a slightly different case just happed for openjdk-20 (which has now been archived last month https://github.com/openjdk/jdk20u)

With my fix the bot created a PR for an old version, the opposite of what was happening with my fix above.

TL;DR I'm going to familiarise myself with the version transform code that was added + get some more test cases that cover these odd versions we're seeing on openjdk.

Leaving as draft for now until I really understand what's going on.

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.

1 participant