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 mike version sorting #171

Merged
merged 7 commits into from
Nov 3, 2023
Merged

Conversation

llucax
Copy link
Contributor

@llucax llucax commented Nov 3, 2023

For the same major and minor, pre-releases were sorted as newer than stable releases, but this is not correct. This PR fixes that.

It also makes the sorting of the versions.json file more robust, as it doesn't parse and re-serialize the version object, but instead only uses the version field for sorting, and the rest of the fields are dumping verbatim without any rewriting.

This means that if new versions of mike add more fields to the JSON, we won't need to update the sorting code, as it will just work.

Signed-off-by: Leandro Lucarella <[email protected]>
For the same major and minor, pre-releases were sorted as newer than
stable releases, but this is not correct.

Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
There are 2 main reasons for this:

1. Parsing the mike versions is less future proof, as if more fields
   are added to the JSON in the future, we'll remove them from the
   output, making the sorting more bug prone and more costly to
   maintain.

2. Avoiding the extra wrapping in `sort_mike_versions()` makes it easier
   to test, as otherwise we need to do the extra wrapping in tests,
   wrapping that is really not related to sorting so it adds noise to
   the tests.

Signed-off-by: Leandro Lucarella <[email protected]>
We are going to add more wrapped test cases, so it is better to use a
more explicit name.

Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
@llucax llucax requested a review from a team as a code owner November 3, 2023 09:58
@llucax llucax requested a review from Marenz November 3, 2023 09:58
@llucax llucax self-assigned this Nov 3, 2023
@github-actions github-actions bot added the part:tests Affects the unit, integration and performance (benchmarks) tests label Nov 3, 2023
@llucax llucax added this to the v0.7.4 milestone Nov 3, 2023
@llucax llucax added type:bug Something isn't working part:ci Affects the GitHub workflow and other parts for running CI part:mkdocs Affects the configuration of mkdocs part:tooling Affects the development tooling (CI, deployment, dependency management, etc.) labels Nov 3, 2023
@llucax llucax added this pull request to the merge queue Nov 3, 2023
Merged via the queue into frequenz-floss:v0.x.x with commit 8bf6c38 Nov 3, 2023
18 checks passed
@llucax llucax deleted the fix-mike-sort branch November 3, 2023 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
part:ci Affects the GitHub workflow and other parts for running CI part:mkdocs Affects the configuration of mkdocs part:tests Affects the unit, integration and performance (benchmarks) tests part:tooling Affects the development tooling (CI, deployment, dependency management, etc.) type:bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants