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

Add human readable yaml ordering #1326

Merged
merged 8 commits into from
Mar 3, 2025
Merged

Conversation

elliotgunton
Copy link
Collaborator

@elliotgunton elliotgunton commented Mar 1, 2025

Pull Request Checklist

Description of PR
Based on YAML being a bunch of nested dictionaries, we can order the nested dictionaries by the name key always-first, then plain data types, then lists, and then dicts, which should go some way towards making Hera-outputted YAMLs much friendlier to DevOps and YAML-first collaborators. As YAML makes no guarantee on dictionary ordering, this change is entirely cosmetic to improve human readability (IMO it really does make the YAML better to read).

Commits are arranged to show changes/fixes per file, then the final commit adds all non-upstream YAMLs (now with nice ordering!)

Signed-off-by: Elliot Gunton <elliotgunton@gmail.com>
* Replace alphabetical ordering in example YAMLs

Signed-off-by: Elliot Gunton <elliotgunton@gmail.com>
Remaining examples will be added in a separate PR.

Signed-off-by: Elliot Gunton <elliotgunton@gmail.com>
@elliotgunton elliotgunton added type:enhancement A general enhancement semver:minor A change requiring a minor version bump labels Mar 1, 2025
Signed-off-by: Elliot Gunton <elliotgunton@gmail.com>
Signed-off-by: Elliot Gunton <elliotgunton@gmail.com>
Copy link

codecov bot commented Mar 1, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.4%. Comparing base (f7ed4b2) to head (421d8b9).
Report is 1 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #1326   +/-   ##
=====================================
  Coverage   86.4%   86.4%           
=====================================
  Files         60      60           
  Lines       4099    4118   +19     
  Branches     651     657    +6     
=====================================
+ Hits        3543    3562   +19     
  Misses       388     388           
  Partials     168     168           

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

@elliotgunton elliotgunton marked this pull request as ready for review March 1, 2025 23:23
Signed-off-by: Elliot Gunton <elliotgunton@gmail.com>
@elliotgunton elliotgunton force-pushed the add-human-readable-yaml-ordering branch from 1f83db1 to ef93806 Compare March 3, 2025 12:08
elliotgunton added a commit that referenced this pull request Mar 3, 2025
* If the upstream examples are changed while working on a feature, it
can be frustrating to regenerate YAML files as they cannot match without
manually editing the Python versions, which mixes up intentions in the
PR.
* For that reason, use separate make targets for regenerating the
Hera-generated YAML files (`regenerate-upstream-test-data`) and fetching
the actual upstream YAML files - `fetch-upstream-examples`.
* Add a check (when `REPORT_DIFFS` env var is set) to ensure that the
local copy of the upstream YAML matches for
`regenerate-upstream-test-data`, with a test fail message to tell user
to fetch the new YAML with the `fetch-upstream-examples` target.
* Update the `regenerate-test-data` target to only regenerate
non-upstream examples.

With this change the new developer workflow will be:
* If making changes to Hera-specific examples (e.g. runner examples),
use `make regenerate-test-data`
* If making changes that affect all examples (e.g. YAML key ordering
#1326) which should rarely happen, use both: `make regenerate-test-data
regenerate-upstream-test-data`
* See if there are changes to upstream example YAMLs that have been
recreated in Hera by running `REPORT_DIFFS make
regenerate-upstream-test-data` (TODO: add an automated way to notify us
of changes)
* If there are new upstream examples or they have changed, run `make
fetch-upstream-examples` to update the `.upstream.yaml` files, then work
on the respective Python files to update the Hera workflow, and then
regenerate the matching `.yaml` files using `make
regenerate-upstream-test-data`

---------

Signed-off-by: Elliot Gunton <elliotgunton@gmail.com>
Signed-off-by: Elliot Gunton <elliotgunton@gmail.com>
Signed-off-by: Elliot Gunton <elliotgunton@gmail.com>
@elliotgunton elliotgunton enabled auto-merge (squash) March 3, 2025 13:28
@elliotgunton elliotgunton merged commit ebb3a49 into main Mar 3, 2025
25 checks passed
@elliotgunton elliotgunton deleted the add-human-readable-yaml-ordering branch March 3, 2025 14:43
@jaseemabid
Copy link
Contributor

Thank you for this 👍

@jaseemabid jaseemabid mentioned this pull request Mar 4, 2025
4 tasks
elliotgunton added a commit that referenced this pull request Mar 4, 2025
WF templates can have list of lists, for example steps.

This PR makes sure those are sorted as well.

Follow up to #1326

---------

Signed-off-by: Jaseem Abid <me@jabid.in>
Signed-off-by: Elliot Gunton <elliotgunton@gmail.com>
Co-authored-by: Elliot Gunton <elliotgunton@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
semver:minor A change requiring a minor version bump type:enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add sensible key-ordering defaults for exporting to YAML
3 participants