-
Notifications
You must be signed in to change notification settings - Fork 207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: capture commands should respect --namespace #568
base: main
Are you sure you want to change the base?
Conversation
@@ -9,7 +9,10 @@ import ( | |||
"k8s.io/cli-runtime/pkg/genericclioptions" | |||
) | |||
|
|||
var name string | |||
var opts = struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can borrow what kubectl cli has been achieved here? Like https://github.com/kubernetes/kubectl/blob/master/pkg/cmd/create/create_cronjob.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^^ it's for this change, we can improve later.
Thanks for fixing missing namespace flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the direction I am moving with this change, but it is just the first small step
Signed-off-by: Evan Baker <[email protected]>
4137260
to
fdb4a33
Compare
opts.AddFlags(capture.PersistentFlags()) | ||
capture.PersistentFlags().StringVar(opts.Name, "name", "", "The name of the Retina Capture") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's a small thing here, but this might be better as a method of opts
. You can inject whatever the value of capture.PersistentFlags()
is and let opts add Flags to it. It'd make it a little more modular by letting the opts say "I know what my flags should be".
Not a blocker, but just an idea.
This PR will be closed in 7 days due to inactivity. |
This PR will be closed in 7 days due to inactivity. |
Description
Fix
capture
subcommands to respect the cli-runtime--namespace
CLI flag.Related Issue
Closes #477
Checklist
git commit -S -s ...
). See this documentation on signing commits.Please refer to the CONTRIBUTING.md file for more information on how to contribute to this project.