Improve drenv CLI help with descriptions and examples - #2688
Improve drenv CLI help with descriptions and examples #2688SuchiBhargav wants to merge 1 commit into
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe ChangesCLI help enhancements
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/drenv/__main__.py`:
- Around line 235-237: Fix the `stress-test run` example in the help text near
the `add_command` registration: include the required environment-file positional
argument in the example, unless `stress.run` is intentionally environment-file
independent, in which case configure its `add_command` call with `envfile=False`
and keep the shorter example.
- Around line 316-328: Rename the add_command parameter from help to help_text
to avoid shadowing Python’s built-in, update all add_command callers to pass
help_text, and preserve the parser configuration by forwarding it as
help=help_text to sp.add_parser.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e89858cd-c3d0-4cf6-b78c-9d5bf1fb917c
📒 Files selected for processing (1)
test/drenv/__main__.py
📜 Review details
🧰 Additional context used
🪛 Ruff (0.16.0)
test/drenv/__main__.py
[error] 320-320: Function argument help is shadowing a Python builtin
(A002)
🔇 Additional comments (1)
test/drenv/__main__.py (1)
144-160: LGTM!Also applies to: 181-192, 201-206, 221-224, 228-234, 238-253, 281-286, 299-304, 329-332, 799-799
| "Examples:\n" | ||
| " drenv stress-test run\n" | ||
| " drenv stress-test report out" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the stress-test run example.
add_command defaults envfile=True, and the run subcommand does not override it. The parser therefore requires a positional envfile. Line 236 omits that argument, so copying the example produces an argparse error.
Use an example that includes the environment file. If stress.run does not need an environment file, set envfile=False in the add_command call instead.
Proposed help-text fix
- " drenv stress-test run\n"
+ " drenv stress-test run envs/regional-dr.yaml\n"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "Examples:\n" | |
| " drenv stress-test run\n" | |
| " drenv stress-test report out" | |
| "Examples:\n" | |
| " drenv stress-test run envs/regional-dr.yaml\n" | |
| " drenv stress-test report out" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/drenv/__main__.py` around lines 235 - 237, Fix the `stress-test run`
example in the help text near the `add_command` registration: include the
required environment-file positional argument in the example, unless
`stress.run` is intentionally environment-file independent, in which case
configure its `add_command` call with `envfile=False` and keep the shorter
example.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/drenv/__main__.py`:
- Around line 799-804: Resolve the conflict at the module entry point in
test/drenv/__main__.py by removing all Git conflict markers and retaining
exactly one main() invocation, so the CLI executes once and the file remains
valid Python.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0e7b69b6-55f4-4f31-887a-c9018a749cc3
📒 Files selected for processing (1)
test/drenv/__main__.py
📜 Review details
⏰ Context from checks skipped due to timeout. (9)
- GitHub Check: Golangci Lint (e2e)
- GitHub Check: Go compatibility
- GitHub Check: Golangci Lint (.)
- GitHub Check: Build image
- GitHub Check: ramendev (ubuntu-24.04, 3.10)
- GitHub Check: Linters
- GitHub Check: Golangci Lint (api)
- GitHub Check: Unit tests
- GitHub Check: ramendev (ubuntu-24.04, 3.13)
🧰 Additional context used
🪛 Ruff (0.16.1)
test/drenv/__main__.py
[warning] 799-799: Expected an indented block after if statement
(invalid-syntax)
[warning] 799-799: Expected a statement
(invalid-syntax)
[warning] 799-799: Expected a statement
(invalid-syntax)
[warning] 799-799: Expected a statement
(invalid-syntax)
[warning] 800-800: Unexpected indentation
(invalid-syntax)
[warning] 801-801: Expected a statement
(invalid-syntax)
[warning] 801-801: Expected a statement
(invalid-syntax)
[warning] 801-801: Expected a statement
(invalid-syntax)
[warning] 801-801: Expected a statement
(invalid-syntax)
[warning] 801-802: Expected a statement
(invalid-syntax)
[warning] 802-802: Unexpected indentation
(invalid-syntax)
[warning] 804-804: Expected a statement
(invalid-syntax)
[warning] 804-804: Expected a statement
(invalid-syntax)
[warning] 804-804: Expected a statement
(invalid-syntax)
[warning] 804-804: Expected a statement
(invalid-syntax)
[warning] 804-804: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
[warning] 804-804: Expected ,, found name
(invalid-syntax)
| <<<<<<< HEAD | ||
| main() | ||
| ======= | ||
| main() | ||
|
|
||
| >>>>>>> 6a991aaa (format updated) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Resolve the merge conflict before merging.
Lines 799-804 contain Git conflict markers, so Python cannot parse test/drenv/__main__.py. Remove the markers and retain exactly one main() call. Keeping both calls would execute the CLI twice.
Proposed fix
-<<<<<<< HEAD
- main()
-=======
- main()
-
->>>>>>> 6a991aaa (format updated)
+ main()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <<<<<<< HEAD | |
| main() | |
| ======= | |
| main() | |
| >>>>>>> 6a991aaa (format updated) | |
| main() |
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 799-799: Expected an indented block after if statement
(invalid-syntax)
[warning] 799-799: Expected a statement
(invalid-syntax)
[warning] 799-799: Expected a statement
(invalid-syntax)
[warning] 799-799: Expected a statement
(invalid-syntax)
[warning] 800-800: Unexpected indentation
(invalid-syntax)
[warning] 801-801: Expected a statement
(invalid-syntax)
[warning] 801-801: Expected a statement
(invalid-syntax)
[warning] 801-801: Expected a statement
(invalid-syntax)
[warning] 801-801: Expected a statement
(invalid-syntax)
[warning] 801-802: Expected a statement
(invalid-syntax)
[warning] 802-802: Unexpected indentation
(invalid-syntax)
[warning] 804-804: Expected a statement
(invalid-syntax)
[warning] 804-804: Expected a statement
(invalid-syntax)
[warning] 804-804: Expected a statement
(invalid-syntax)
[warning] 804-804: Expected a statement
(invalid-syntax)
[warning] 804-804: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
[warning] 804-804: Expected ,, found name
(invalid-syntax)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/drenv/__main__.py` around lines 799 - 804, Resolve the conflict at the
module entry point in test/drenv/__main__.py by removing all Git conflict
markers and retaining exactly one main() invocation, so the CLI executes once
and the file remains valid Python.
Source: Linters/SAST tools
99822e4 to
0ebcca7
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Improve the help output by adding detailed command descriptions, usage information, and examples for drenv subcommands. This makes the CLI easier to learn and reduces the need to consult external documentation for common workflows. Signed-off-by: Suchi <hiiamsuchi@gmail.com>
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
| help=None, | ||
| envfile=True, | ||
| description=None, | ||
| epilog=None, |
There was a problem hiding this comment.
Added after the options - we want to add the example before - see the issue.
| func, | ||
| help=None, | ||
| envfile=True, | ||
| description=None, |
There was a problem hiding this comment.
This should be the only change - the caller will add the long description and the examples.
| envfile=True, | ||
| description=None, | ||
| epilog=None, | ||
| formatter_class=argparse.HelpFormatter, |
There was a problem hiding this comment.
We don't want that every command will have to set the formatter_calss. Use this internally transparently.
| "Compare the results of two stress-test output directories and\n" | ||
| "summarize the differences." | ||
| ), | ||
| formatter_class=argparse.RawDescriptionHelpFormatter, |
There was a problem hiding this comment.
This is can be use internally for all commands, or for commands with description != None
| stress.compare, | ||
| help="compare 2 stress tests", | ||
| envfile=False, | ||
| description="Compare two stress test runs.", |
There was a problem hiding this comment.
This does not add new content - same as the help=
| epilog=( | ||
| "Compare the results of two stress-test output directories and\n" | ||
| "summarize the differences." | ||
| ), |
| "comparison and reporting.\n\n" | ||
| "Examples:\n" | ||
| " drenv stress-test run\n" | ||
| " drenv stress-test report out" |
There was a problem hiding this comment.
Using multiple lines with "\n" is messy. We can use:
description="""
Execute repeated stress-test runs and collect results for later comparison and reporting.
Examples:
# Description of this example
drenv stress-test run
# Description of this example
drenv stress-test report out
""".strip()
| " drenv stress-test run\n" | ||
| " drenv stress-test report out" | ||
| ), | ||
| formatter_class=argparse.RawDescriptionHelpFormatter, |
There was a problem hiding this comment.
Test how this deals with long lines. We want the formatted to handle line wrapping, keeping existing structure.
3 participants
Summary
Improve the
drenvCLI help output by adding descriptive text andusage examples to make commands easier to understand and use.
Changes
Why
The previous help output primarily displayed the command syntax and
available options, requiring users to refer to external documentation
to understand a command's purpose and usage. This change makes the
built-in help more informative and aligns it more closely with the
user experience provided by mature CLIs such as
kubectl.Example
Fixes #2690