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

CRAYSAT-1551: Fix sorting of product versions in sat showrev --products #287

Merged
merged 1 commit into from
Nov 21, 2024

Conversation

ethanholen-hpe
Copy link
Contributor

@ethanholen-hpe ethanholen-hpe commented Nov 13, 2024

Summary and Scope

Fixed the sorting of product versions for sat showrev --products as outlined in CRAYSAT-1551

Issues and Related PRs

Testing

Tested on:

Test description:

Tests were run on drax and tested locally, unit tests were also added for the new class.

Risks and Mitigations

Pull Request Checklist

  • Version number(s) incremented, if applicable
  • Copyrights updated
  • License file intact
  • Target branch correct
  • CHANGELOG.md updated
  • Testing is appropriate and complete, if applicable
  • HPC Product Announcement prepared, if applicable

@ethanholen-hpe ethanholen-hpe changed the title Fix sorting of product versions in sat showrev --products CRAYSAT-1551: Fix sorting of product versions in sat showrev --products Nov 14, 2024
@ethanholen-hpe ethanholen-hpe marked this pull request as ready for review November 19, 2024 13:59
@ethanholen-hpe
Copy link
Contributor Author

Output from testing on Drax with correctly ordered versions
https://gist.github.com/ethanholen-hpe/6ac403f4ddff61cb0c03d66809ec5450

Copy link
Contributor

@haasken-hpe haasken-hpe left a comment

Choose a reason for hiding this comment

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

Mostly minor comments.

However, using --format json and --format yaml is broken and needs to be fixed. It should be a simple fix as we discussed. Take a look at how the XName class is handled in the SATEncoder and in SATDumper in sat.util.

sat/loose_version.py Outdated Show resolved Hide resolved
sat/loose_version.py Outdated Show resolved Hide resolved
sat/loose_version.py Outdated Show resolved Hide resolved
sat/loose_version.py Outdated Show resolved Hide resolved
sat/loose_version.py Outdated Show resolved Hide resolved
tests/test_loose_version.py Outdated Show resolved Hide resolved
tests/test_loose_version.py Outdated Show resolved Hide resolved
tests/test_loose_version.py Outdated Show resolved Hide resolved
tests/test_loose_version.py Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@ethanholen-hpe
Copy link
Contributor Author

Output from drax with functional --format json and yaml
https://gist.github.com/ethanholen-hpe/30878bece01d6808c25a7eb2147a3bc7

Copy link
Contributor

@haasken-hpe haasken-hpe left a comment

Choose a reason for hiding this comment

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

Minor comments.

CHANGELOG.md Outdated Show resolved Hide resolved
tests/test_loose_version.py Show resolved Hide resolved
@ethanholen-hpe ethanholen-hpe merged commit a24e0dd into main Nov 21, 2024
3 checks passed
@ethanholen-hpe ethanholen-hpe deleted the CRAYSAT-1551 branch November 21, 2024 17:57
@ethanholen-hpe
Copy link
Contributor Author

/backport release/3.32

Copy link

Backporting into branch release/3.32 was successful. New PR: #290

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.

2 participants