fix: command in karmadactl create should be karmadactl instead of kubectl#7545
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR adds richer create subcommand help output by introducing per-subcommand Example strings and wiring a recursive helper to apply them across the cobra command tree.
Changes:
- Added example text blocks for many
createsubcommands (namespace, quota, configmap, RBAC, ingress, secrets, services, etc.). - Introduced a
createSubcommandExampleslookup map keyed by subcommand name. - Added
setCreateSubcommandExamplesto recursively setsubCmd.ExampleduringNewCmdCreate.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive set of example strings for various karmadactl create subcommands and implements a recursive function, setCreateSubcommandExamples, to apply these examples to the command hierarchy. Feedback was provided regarding a formatting bug in the podDisruptionBudgetExample where a percentage sign needs to be escaped to prevent fmt.Sprintf errors. Additionally, it was recommended to add a documentation comment to the new setCreateSubcommandExamples function to align with maintainability best practices.
53acb8d to
57604bd
Compare
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7545 +/- ##
==========================================
- Coverage 42.07% 42.06% -0.01%
==========================================
Files 879 879
Lines 54821 54827 +6
==========================================
- Hits 23064 23063 -1
- Misses 30013 30020 +7
Partials 1744 1744
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Just out of curiosity, the generated command-line-flags should be updated after this PR, but why not? |
because at that time, karmadactl doc 's PR had not yet been merged lol.Let me rebase to the newest |
|
OK. thanks. |
b0f4fc0 to
9674d1c
Compare
|
/cc @RainbowMango I simplified it to a recursive method. |
| func replaceCreateSubcommandExamples(cmd *cobra.Command, parentCommand string) { | ||
| for _, subCmd := range cmd.Commands() { | ||
| if subCmd.Example != "" { | ||
| subCmd.Example = strings.ReplaceAll(subCmd.Example, "kubectl", parentCommand) |
There was a problem hiding this comment.
Another replace approach is to strictly match kubectl create, so it won't accidentally replace anything should't be replaced, like:
| subCmd.Example = strings.ReplaceAll(subCmd.Example, "kubectl", parentCommand) | |
| subCmd.Example = strings.ReplaceAll(subCmd.Example, "kubectl create", parentCommand+" create") |
But, no big deal, it's up to you.
/approve
/lgtm
/hold
for @FAUST-BENCHOU confirm
…ectl Signed-off-by: zhoujinyu <2319109590@qq.com>
9674d1c to
1e78543
Compare
RainbowMango
left a comment
There was a problem hiding this comment.
/lgtm
/approve
/hold cancel
Thanks.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: RainbowMango The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #7542
Special notes for your reviewer:
Does this PR introduce a user-facing change?: