Add --context support #1
Conversation
| @@ -8,7 +8,7 @@ describe() { | |||
| help() { | |||
| echo -e " | |||
| Usage: | |||
| forward [-n namespace] host [sport:]dport | |||
| forward [--context context] [-n namespace] host [sport:]dport | |||
There was a problem hiding this comment.
Let's keep this code consistent and use short options. I would suggest -c context here.
There was a problem hiding this comment.
Just wanted to be in line with other built-in kubectl commands here. There's just --context available.
There was a problem hiding this comment.
I understand, nevertheless using short options is cleaner here imho. We already do it for the namespace which is also supported natively by the kubectl. So, Iwould either:
- change the
contextto justcto stay consistent with the current style, - or remove it completely in order to allow users to pass any
kubectlbuilt-in options, but without creating an explicits support for them here. In this scenario I would leave a note in readme to let users know they could find these options with thekubectl options.
There was a problem hiding this comment.
I see. But it's a bit different with --namespace as kubectl also supports -n as alias for it while -c isn't an alias for --context but is used to specify a config file.
The latter probably would be the approach here but I don't have time to work on this right now.
There was a problem hiding this comment.
Sure, no worries, there is no rush, feel free to come back to it when you have more time.
There was a problem hiding this comment.
I'll keep using my fork for now.
| case "$opt" in | ||
| h) | ||
| describe | ||
| help | ||
| ;; | ||
| n) namespace="$OPTARG" | ||
| ;; | ||
| -) |
There was a problem hiding this comment.
Same as above, let's keep it consistent and simplify by using short option.
|
@levrik direction is good, I have minor comments though |
No description provided.