8000 Update Cobra to version 1.2.1 by marckhouzam · Pull Request #9933 · helm/helm · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Update Cobra to version 1.2.1 #9933

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

Merged
merged 2 commits into from
Jul 14, 2021
Merged

Conversation

marckhouzam
Copy link
Member

What this PR does / why we need it:

Update to Cobra 1.2.1.
I don't know why Dependabot was not triggered for this.

The changes part of Cobra 1.2.1 are safe and are mostly focused on bug fixes for the completion support.

A bash completion V2 is also available. I will eventually post a PR proposing to use this new version.

Special notes for your reviewer:

A completion test was removed in this PR as it was failing with the new version of Cobra.
The test was actually testing Cobra's behaviour (which changed in the new release), which is not something Helm tests should focus on, so I simply removed it.

I don't expect Cobra's change of behaviour to have much of an impact on Helm, and I was being over-zealous when I wrote the test.

The completion tests of the acceptance-testing repo continue to pass.

Should I be checking the changes to go.sum in any way?

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
Testing that a bad completion directive was being replaced by the
default one was actually testing Cobra's behaviour.  This is unnecessary
especially since that behaviour changed in the 1.2.0 release.  Helm
tests should focus on testing Helm's behaviour.

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
@helm-bot helm-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 11, 2021
@marckhouzam
Copy link
Member Author

Here are the release notes for the 1.2.0 and 1.2.1 release of Cobra, if anyone is curious.

@mattfarina mattfarina added this to the 3.7.0 milestone Jul 12, 2021
Comment on lines -280 to -284
}, {
name: "completion for plugin bad directive",
cmd: "__complete echo ''",
golden: "output/plugin_echo_bad_directive.txt",
rels: []*release.Release{},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, why was this test dropped?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(From the description of the PR 😄 ):

A completion test was removed in this PR as it was failing with the new version of Cobra.
The test was actually testing Cobra's behaviour (which changed in the new release), which is not something Helm tests should focus on, so I simply removed it.

I don't expect Cobra's change of behaviour to have much of an impact on Helm, and I was being over-zealous when I wrote the test.

Copy link
Contributor
@hickeyma hickeyma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @marckhouzam.

@marckhouzam marckhouzam merged commit 6a3daaa into helm:main Jul 14, 2021
@marckhouzam marckhouzam deleted the feat/cobra121 branch July 14, 2021 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0