Little kubectl improvements#2227
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b156c2762
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| cube version: "kubectl version " | ||
|
|
||
| cube {user.kubectl_action} [{user.kubectl_object}]: | ||
| cube {user.kubectl_action} [{user.kubectl_object}] [{user.kubectl_output_format_object}]: |
There was a problem hiding this comment.
Restrict output formats to commands that support them
Because the optional output format is attached to the generic {user.kubectl_action} rule, phrases such as “cube describe pods yaml” now emit kubectl describe pods -o yaml. Checked the generated Kubernetes kubectl docs: kubectl get explicitly supports (-o|--output=)... output formats, while kubectl describe's synopsis/options list no output flag (https://kubernetes.io/docs/reference/kubectl/generated/kubectl_get/ and https://kubernetes.io/docs/reference/kubectl/generated/kubectl_describe/). This makes valid-sounding describe/delete/label voice commands generate unsupported kubectl invocations; the formatter should be limited to get or to actions/formats that actually support it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
That is correct. Suggest a fix
There was a problem hiding this comment.
To use Codex here, create a Codex account and connect to github.
There was a problem hiding this comment.
I know that git does not do such sanity checks to. I would say unless there is found a very nice way to do this, to not do the sanity check
There was a problem hiding this comment.
Since the git integration also has this problem I would suggest accepting that until a nice solutions becomes clear
There was a problem hiding this comment.
From the community backlog session — agree that it's not necessary to filter this out unless you find an elegant way to do so.
| cube {user.kubectl_action} [{user.kubectl_object}] [{user.kubectl_output_format_object}]: | ||
| insert("kubectl {kubectl_action} ") | ||
| insert(kubectl_object or "") | ||
| # requires beta: |
There was a problem hiding this comment.
Can you change this to not require beta? You could implement this command with an action instead doing the if statement in Python. You could also try using the or syntax, as is done in line 62 of this file.
Example of or from the wiki:
# Saying "defaultable plex" inserts "x", saying "defaultable" inserts "default"
defaultable [<user.letter>]:
insert(letter or "default")
There was a problem hiding this comment.
From the community backlog session — right now we do not require the beta for any functionality. This should be easy enough to fix!
| ) | ||
|
|
||
| mod.list("kubectl_output_format_object", desc="-o parameters, so output formatters") | ||
| ctx.lists["self.kubectl_output_format_object"] = ("yaml", "wide") |
There was a problem hiding this comment.
As explained in https://github.com/talonhub/community/blob/main/CONTRIBUTING.md#coding-principles P05, we typically implement lists in .talon-list files unless we have a good reason to do otherwise to make it easier for users to customize. Some users might want to change the spoken forms. Would you mind migrating this to a .talon-list and either migrating the preexisting list or filing an issue for that?
There was a problem hiding this comment.
Will do when I have some time.
| cube {user.kubectl_action} [{user.kubectl_object}] [{user.kubectl_output_format_object}]: | ||
| insert("kubectl {kubectl_action} ") | ||
| insert(kubectl_object or "") | ||
| # requires beta: |
There was a problem hiding this comment.
From the community backlog session — right now we do not require the beta for any functionality. This should be easy enough to fix!
| cube version: "kubectl version " | ||
|
|
||
| cube {user.kubectl_action} [{user.kubectl_object}]: | ||
| cube {user.kubectl_action} [{user.kubectl_object}] [{user.kubectl_output_format_object}]: |
There was a problem hiding this comment.
From the community backlog session — agree that it's not necessary to filter this out unless you find an elegant way to do so.
See commits
might push more in the next weeks in the same PR unless you say I should not
note that at least one of them require beta