Skip to content
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

vendor: bump cobra to v1.1.1 #391

Merged
merged 2 commits into from
Oct 19, 2020
Merged

vendor: bump cobra to v1.1.1 #391

merged 2 commits into from
Oct 19, 2020

Conversation

twpayne
Copy link
Contributor

@twpayne twpayne commented Oct 18, 2020

cobra 1.1.0 inadvertently included a breaking YAML change affecting
kubectl and helm. See spf13/cobra#1259 for
details.

Signed-off-by: Tom Payne tom@isovalent.com

@twpayne twpayne requested a review from rolinh October 18, 2020 19:29
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label PR is blocked until the release note is set label Oct 18, 2020
@twpayne twpayne added the release-note/misc This PR makes changes that have no direct user impact. label Oct 18, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label PR is blocked until the release note is set label Oct 18, 2020
Copy link
Member

@rolinh rolinh left a comment

Choose a reason for hiding this comment

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

I think this won't be enough as our go.mod still points to gopkg.in/yaml.v2 v2.3.0 (ie the version with the unintended breaking change that cobra v1.1.1 reverted). Please, also downgrade the yaml to gopkg.in/yaml.v2 v2.2.8.

@sayboras
Copy link
Member

I think this won't be enough as our go.mod still points to gopkg.in/yaml.v2 v2.3.0 (ie the version with the unintended breaking change that cobra v1.1.1 reverted). Please, also downgrade the yaml to gopkg.in/yaml.v2 v2.2.8.

Might need to use replace directive to enforce v2.2.8 as well. Thinking about it, I noticed that cilium is using v2.3.0, is there anything to worry about? /cc @rolinh @aanm

@twpayne twpayne force-pushed the pr/twpayne/bump-cobra branch from 9ca542f to 78c1b60 Compare October 19, 2020 08:44
gopkg.in/yaml.v2 version 2.3.0 changes the default line wrapping from 80
characters to none. This potentially breaks Kubernetes and Helm. See
spf13/cobra#1259 for discussion.

Signed-off-by: Tom Payne <tom@isovalent.com>
Signed-off-by: Tom Payne <tom@isovalent.com>
@twpayne twpayne force-pushed the pr/twpayne/bump-cobra branch from 78c1b60 to 1ae702e Compare October 19, 2020 08:47
@twpayne
Copy link
Contributor Author

twpayne commented Oct 19, 2020

Thanks for the feedback, PR updated to pin gopkg.in/yaml.v2 to v2.2.8 as suggested by @sayboras.

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 19, 2020
Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

LGTM 💯

Seems like I have permission in hubble as well, I thought it's only cilium repo :)

@twpayne twpayne merged commit 4638263 into master Oct 19, 2020
@twpayne twpayne deleted the pr/twpayne/bump-cobra branch October 19, 2020 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants