Preserve folder structure when using CLI code conversion; add --flatten flag#1443
Merged
Conversation
elliotgunton
added
semver:patch
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1443 +/- ##
=======================================
+ Coverage 87.6% 87.8% +0.1%
=======================================
Files 62 62
Lines 4456 4447 -9
Branches 735 733 -2
=======================================
+ Hits 3905 3906 +1
+ Misses 390 383 -7
+ Partials 161 158 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
elliotgunton
force-pushed
the
refactor-cli
branch
from
07b9f3e to
eccb0dc
Compare
* Extract file interactions into `util.py` * Use `Path` functions directly where possible * Simplify test_generate_yaml to use `tmp_path` instead of `os` mock_os * Reword CLI help Signed-off-by: Elliot Gunton <elliotgunton@gmail.com>
* Recursive traversal will now match the input folder structure for the output by default_extension * Users may still want to flatten the output (e.g. output everything to `manifests/`), so add the explicit `--flatten` flag * Only works for `generate yaml`, needs refactor for `generate python` to use the same logic Signed-off-by: Elliot Gunton <elliotgunton@gmail.com>
* Extract logic to `convert_code` function in `util.py` * Add `--flatten` flag to GeneratePython * Add tests in test_generate_python that are equivalent to the test_generate_yaml tests Signed-off-by: Elliot Gunton <elliotgunton@gmail.com>
Signed-off-by: Elliot Gunton <elliotgunton@gmail.com>
elliotgunton
force-pushed
the
refactor-cli
branch
from
eccb0dc to
1654f65
Compare
alicederyn
approved these changes
Jul 22, 2025
alicederyn
left a comment
alicederyn
left a comment
Collaborator
There was a problem hiding this comment.
Seems sensible! Just some nits, up to you if you want to address them now.
| pytest.mark.xfail( | ||
| reason="Multiple workflows in one yaml file not yet supported.\nYAML round trip issues for certain types." | ||
| reason="Multiple workflows in one yaml file not yet supported.\nYAML round trip issues for certain types.", | ||
| strict=True, |
Comment on lines
+58
to
+63
| flatten: Annotated[ | ||
| bool, | ||
| Arg( | ||
| help="If 'to' is a folder and you have specified '--recursive', then use 'flatten' to output to YAML files without matching the input directory structure. No effect if 'to' is a file (all Workflows will be output in one file)." | ||
| ), | ||
| ] = False |
Collaborator
There was a problem hiding this comment.
Do we think this is worth the maintenance cost?
Collaborator
Author
There was a problem hiding this comment.
I think it's worth it just as a feature - e.g. if I have workflows all over my module as a user, and I want to output them all to manifests/ as a flat folder, I'd get annoyed if I had to mess about with shell commands to do all the mv manifests/a/b/c.py manifests/c.py (unless I'm missing a really obvious way to do that, I feel like mv manifests/**/*.py manifests/ wouldn't do the right thing?)
* Also tidy up based on PR comments Signed-off-by: Elliot Gunton <elliotgunton@gmail.com>
elliotgunton
force-pushed
the
refactor-cli
branch
from
9aa2ebf to
9dd8cc4
Compare
elliotgunton
changed the title
--flatten flag
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Checklist
Description of PR
This is mostly a refactor of the CLI code with a small feature added. The
--recursiveflag now by default preserves the input directory structure when outputting to a folder, i.e. they will have the same structure. The--flattenflag was added to allow the old behaviour, which is still useful for outputting many files to a single, flat folder.In terms of refactors:
util.pyPathfunctions directly where possibletmp_pathinstead ofosmock_osconvert_codefunction inutil.pyAlso added all the equivalent YAML-to-Python CLI tests as the (simplified) test_generate_yaml tests.