-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
🌱 Enhance Subcommand API #1936
🌱 Enhance Subcommand API #1936
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Adirio The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ac34765
to
2c4d921
Compare
2c4d921
to
3e844cc
Compare
6325c25
to
bc55bfd
Compare
…put Subcommand.UpdateMetadata Signed-off-by: Adrian Orive <[email protected]>
Signed-off-by: Adrian Orive <[email protected]>
Signed-off-by: Adrian Orive Oneca <[email protected]>
bc55bfd
to
bf95cc4
Compare
#1980 implements this with additional cleanups, please consider merging the other PR first (which will close this one) unless any of the other changes there are blocking and we want to merge this first. |
@Adirio: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Description
Subcommand.UpdateContext
method was being used for both inserting cli related metadata to the subcommands and returning subcommand related metadata to the command constructor (cli). TheContext
struct didn't have anything to do with Go'scontext
pacvkage which could also lead to harder to read code.This PR uses the term
Metadata
instead ofContext
(which also has the advantage of not conflicting withonsi/ginkgo
) and makes clearer differentioation between the input and output metadatas (cli->subcommand and subcommand->cli).