add create command [AI]#1601
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new create command to Ramble, enabling users to generate application, modifier, and other object definitions either interactively or via the CLI. It includes the command-line interface, a creation backend, starter templates, bash completion updates, and unit tests. Feedback focuses on improving robustness and correctness: handling NoRepoConfiguredError when base class repositories are missing, preventing the interactive wizard from hanging in non-interactive environments, explicitly handling the 'builtin' repository fallback, ensuring consistent UTF-8 file encoding, correcting double curly brace escaping in templates to prevent formatting errors, and fixing a typo in the bash completion script.
Ramble Performance Test MetricsResults produced with commit: 3597362
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1601 +/- ##
===========================================
+ Coverage 93.02% 93.06% +0.04%
===========================================
Files 348 351 +3
Lines 33858 34232 +374
===========================================
+ Hits 31497 31859 +362
- Misses 2361 2373 +12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
rfbgo
force-pushed
the
create
branch
3 times, most recently
from
b81962a to
342a807
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new create command in Ramble, allowing users to generate application, modifier, system, and other object definition stubs either via command-line arguments or an interactive wizard. It also updates bash completion, adds templates, fixes a completion typo, and updates package manager logic to evaluate conditional required packages. Feedback on these changes includes filtering out empty values from user-provided maintainers and tags, handling EOFError gracefully in the interactive wizard, setting the correct default base class for package managers, and safely retrieving the when clause in the user-managed package manager to avoid potential KeyErrors.
douglasjacobsen
left a comment
douglasjacobsen
left a comment
There was a problem hiding this comment.
Thanks, this is great. I have a few comments, but otherwise it looks good.
| ) | ||
| subparser.add_argument( | ||
| "-r", | ||
| "--repo", |
There was a problem hiding this comment.
I wonder how hard it would be to support something like:
ramble create builtin.app.foo
And have the repo be picked (and the type) automatically from that.
|
|
||
| def run_interactive_wizard(): | ||
| """Runs the step-by-step interactive wizard.""" | ||
| print("\n" + "=" * 40) |
There was a problem hiding this comment.
Does it make sense (I'm not sure...) for these to be logger calls instead of direct prints?
| assert create_cmd.returncode in (None, 0) | ||
|
|
||
|
|
||
| def test_create_application_and_modifier(mutable_config, tmpdir): |
There was a problem hiding this comment.
I wonder if the create tests can either be parameterized, or just collapsed into a single one?
| "type": ramble.keywords.key_type.required, | ||
| "level": ramble.keywords.output_level.variable, | ||
| for pkgname, config in obj.required_packages.items(): | ||
| if app_inst.expander.satisfies( |
There was a problem hiding this comment.
I don't think this belongs in this PR, but this might be where you were running into this issue.
2 participants
No description provided.