Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Human sort nested lists #1333

Merged
merged 2 commits into from
Mar 4, 2025

Conversation

jaseemabid
Copy link
Contributor

@jaseemabid jaseemabid commented Mar 4, 2025

WF templates can have list of lists, for example steps.

This PR makes sure those are sorted as well.

Ref #1326

Pull Request Checklist

Description of PR

I really haven't tested this throughly, but since you wrote the code only yesterday I'm guessing you would still have context @elliotgunton.

I can clean up, add some tests etc later when I get some time. Leaving this here as a draft for now.

@jaseemabid jaseemabid force-pushed the jabid/sort-nested-lists branch from eac701f to 5714275 Compare March 4, 2025 13:33
@elliotgunton elliotgunton added semver:patch A change requiring a patch version bump type:enhancement A general enhancement labels Mar 4, 2025
@elliotgunton
Copy link
Collaborator

Thanks @jaseemabid! It LGTM, I will just push the updated examples collection to the branch before merging 🚀

WF templates can have list of lists, for example steps.

This PR makes sure those are sorted as well.

Signed-off-by: Jaseem Abid <me@jabid.in>
@elliotgunton elliotgunton force-pushed the jabid/sort-nested-lists branch from 5714275 to 2588ce5 Compare March 4, 2025 13:52
Signed-off-by: Elliot Gunton <elliotgunton@gmail.com>
@elliotgunton elliotgunton enabled auto-merge (squash) March 4, 2025 13:54
Copy link

codecov bot commented Mar 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.5%. Comparing base (632b06c) to head (2d93b79).
Report is 3 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #1333   +/-   ##
=====================================
  Coverage   86.5%   86.5%           
=====================================
  Files         60      60           
  Lines       4123    4127    +4     
  Branches     657     659    +2     
=====================================
+ Hits        3567    3571    +4     
  Misses       388     388           
  Partials     168     168           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@elliotgunton elliotgunton merged commit c84a0fa into argoproj-labs:main Mar 4, 2025
23 checks passed
@jaseemabid
Copy link
Contributor Author

Thank you! I was just about to update the PR with updated examples, took a while to read through the Makefile and figure it out.

@jaseemabid jaseemabid mentioned this pull request Mar 4, 2025
4 tasks
elliotgunton pushed a commit that referenced this pull request Mar 4, 2025
Just had to run `make regenerate-test-data` once, we probably forgot it
last time.

**Pull Request Checklist**
- [X] Fixes #<!--issue number goes here-->
- [X] Tests added
- [x] Documentation/examples added
- [X] [Good commit messages](https://cbea.ms/git-commit/) and/or PR
title

**Description of PR**

Leftover from #1333

Signed-off-by: Jaseem Abid <me@jabid.in>
@elliotgunton elliotgunton added type:skip-changelog A change that does not require a changelog entry and removed semver:patch A change requiring a patch version bump type:enhancement A general enhancement labels Mar 5, 2025
@alicederyn
Copy link
Collaborator

Just got our first internal PR that included this reordering, very nice ❤️

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
type:skip-changelog A change that does not require a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants