Remove the install/delete sub-commands - #59
Conversation
Signed-off-by: Josef Karasek <karasek.jose@gmail.com>
There was a problem hiding this comment.
🟡 Changes recommended
The updated CLI usage string still advertises a (h)elp subcommand even though only -h|--help is supported, which is misleading user-facing documentation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Removes the deprecated install subcommand from the kubectl kedify plugin now that non-Helm installation flows are no longer supported.
Changes:
- Removed the
install/icommand entry from the README core commands list. - Deleted the
install()implementation and removed its dispatch, help text, examples, and related env-var documentation from the mainkubectl-kedifyscript.
File summaries
| File | Description |
|---|---|
| README.md | Updates command documentation to drop the install subcommand. |
| kubectl-kedify | Removes the install subcommand implementation and its references from CLI dispatch and usage/help output. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟢 Approval recommended
The install command removal appears complete and consistent across code and docs, with only minor help-text polish remaining.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
kubectl-kedify:278
- The usage synopsis omits the supported version flag. Since
main()handles-v|--version, include it in the Usage line so users discover it alongside-h|--help.
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
| ### Core Commands | ||
|
|
||
| - **install, i** - Installs the Kedify agent | ||
| - **delete, d** - Uninstalls the Kedify agent |
There was a problem hiding this comment.
nit: do we need to remove "delete" too?
There was a problem hiding this comment.
in fact we should, deleted in later commit
There was a problem hiding this comment.
🟡 Changes recommended
README and Krew metadata still advertise the removed installation capability.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🟢 Approval recommended
The removal is complete and self-contained, with no remaining references to the deleted command, function, or variables anywhere in the repository.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Signed-off-by: Josef Karasek <karasek.jose@gmail.com>
There was a problem hiding this comment.
🟡 Changes recommended
The change also removes the undocumented delete command beyond the stated install scope, and the .krew.yaml short description still advertises the removed install functionality.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
josefkarasek
changed the title
install sub-commandinstall/delete sub-commands
Signed-off-by: Josef Karasek <karasek.jose@gmail.com>
We no-longer support non-helm installation methods