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
Update cobra to 1.2.1 #103448
Update cobra to 1.2.1 #103448
Conversation
/cc @marckhouzam |
@eddiezane: GitHub didn't allow me to request PR reviews from the following users: marckhouzam. Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
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. |
@eddiezane: The
Use
In response to this:
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. |
/test all |
👍 |
@@ -6,6 +6,8 @@ Please refer to [Shell Completions](shell_completions.md) for details. | |||
|
|||
For backward compatibility, Cobra still supports its legacy dynamic completion solution (described below). Unlike the `ValidArgsFunction` solution, the legacy solution will only work for Bash shell-completion and not for other shells. This legacy solution can be used along-side `ValidArgsFunction` and `RegisterFlagCompletionFunc()`, as long as both solutions are not used for the same command. This provides a path to gradually migrate from the legacy solution to the new solution. | |||
|
|||
**Note**: Cobra's default `completion` command uses bash completion V2. If you are currently using Cobra's legacy dynamic completion solution, you should not use the default `completion` command but continue using your own. |
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.
Can you explain what this means? Maybe not in the doc but I don't know what it means and trying to understand.
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.
Cobra 1.2 automatically provides a completion
command for programs that don't provide one themselves. This greatly reduces the barrier-to-entry for providing completion support. This "default" completion command uses the new bash completion v2 logic, also introduced in Cobra 1.2. The v2 bash completion logic only supports custom completions written in Go (which kubectl started using through #96087 ).
But in any case, kubectl is not affected as it already defines its own completion
command which still points to the bash completion v1 logic.
Please refer to https://github.com/spf13/cobra/blob/master/shell_completions.md#bash-completion-v2
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.
the failures in the dependencies presubmit need resolving, looks like other stray changes generated due to that.
@@ -18,6 +18,7 @@ require ( | |||
) | |||
|
|||
replace ( | |||
github.com/spf13/viper => github.com/spf13/viper v1.7.0 |
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 don't think this is expected. You probably need to run the dependency lint script after fixing the other issues
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 don't think this is expected. You probably need to run the dependency lint script after fixing the other issues
I've been running the scripts for the past few hours 😅. This is going to be a bit messy due to the viper upgrade.
/remove-sig api-machinery |
This is ready for review. The added transitive deps are:
/assign @liggitt /remove-area apiserver |
Can confirm this fixes kubernetes/kubectl#1096. |
Fewer dependencies, but more max depth. |
/approve I agree this is a net gain, though the cobra → viper → afero subgraph is tending to pull in additional transitive dependencies with each bump I'd like to avoid adding to the graph ... could be worth looking into isolating / making those optional in the future |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eddiezane, liggitt The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
This PR upgrades our version of cobra to 1.2.1. 1.2.0 overhauled completions.
Which issue(s) this PR fixes:
Fixes kubernetes/kubectl#1096
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: