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

Fixes foojay action, which was missing some updates #676

Merged
merged 1 commit into from
May 9, 2022
Merged

Conversation

dmikusa
Copy link
Contributor

@dmikusa dmikusa commented May 6, 2022

Summary

Corrects the following issues:

  • Versions with a fourth digit were skipped, like 11.0.14.1
  • Versions were incorrectly sorted [1], now using the latest field to pick the latest version
  • If an invalid distro was set, it would work and the API is not validating. It just does not filter at all, which returns all versions and confusing results. This PR validates the distro entered and panics if its bad.

[1] - This happens due to some legacy issues in how we manage semver. If a version is not semver or if it's semver but has a pre-release or post-release number, then we squash all that to the format major.minor.patch-pre which isn't correct but changing it would require a lot of investigation to track down what it might possibly impact. At any rate, because we do this technically 11.0.14-1 is less than 11.0.14 and so 11.0.14 is picked despite what we really want being 11.0.14-1 (which was 11.0.14.1).

Use Cases

Checklist

  • I have viewed, signed, and submitted the Contributor License Agreement.
  • I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
  • I have added an integration test, if necessary.
  • I have reviewed the styleguide for guidance on my code quality.
  • I'm happy with the commit history on this PR (I have rebased/squashed as needed).

Corrects the following issues:

- Versions with a fourth digit were skipped, like 11.0.14.1
- Versions were incorrectly sorted [1], now using the latest field to pick the latest version
- If an invalid distro was set, it would work and the API is not validating. It just does not filter at all, which returns all versions and confusing results. This PR validates the distro entered and panics if its bad.

[1] - This happens due to some legacy issues in how we manage semver. If a version is not semver or if it's semver but has a pre-release or post-release number, then we squash all that to the format `major.minor.patch-pre` which isn't correct but changing it would require a lot of investigation to track down what it might possibly impact. At any rate, because we do this technically 11.0.14-1 is less than 11.0.14 and so 11.0.14 is picked despite what we really want being 11.0.14-1 (which was 11.0.14.1).

Signed-off-by: Daniel Mikusa <dmikusa@vmware.com>
@dmikusa dmikusa added type:bug A general bug semver:patch A change requiring a patch version bump labels May 6, 2022
@dmikusa dmikusa requested a review from a team May 6, 2022 18:45
@dmikusa dmikusa merged commit 662faa4 into main May 9, 2022
@dmikusa dmikusa deleted the foojay-fix branch May 9, 2022 18:06
This was referenced May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:patch A change requiring a patch version bump type:bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants