Skip to content

completion: add wrapper code to transform bash to zsh completion #1685

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
Feb 25, 2019

Conversation

corneliusweig
Copy link
Contributor

Cobra zsh completion is pretty broken. The idea is therefore, to use the working bash completion from cobra and wrap that with zsh adapter code. This code is based on the kubectl wrapper script.

I'm a bit unsure about the proper copyright notice. I left it as-is, but the beef of the logic comes from https://github.com/kubernetes/kubernetes/blob/master/pkg/kubectl/cmd/completion/completion.go . Must I therefore include a proper attribution for the wrapper script?

Fixes #1671

Cobra zsh completion is pretty broken. The idea is therefore, to use the working bash completion from cobra and wrap that with zsh adapter code. This code is based on the kubectl wrapper script.
Fixes GoogleContainerTools#1671

Signed-off-by: Cornelius Weig <[email protected]>
@codecov-io
Copy link

codecov-io commented Feb 22, 2019

Codecov Report

Merging #1685 into master will decrease coverage by 0.2%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1685      +/-   ##
==========================================
- Coverage   46.69%   46.48%   -0.21%     
==========================================
  Files         123      123              
  Lines        5600     5625      +25     
==========================================
  Hits         2615     2615              
- Misses       2716     2741      +25     
  Partials      269      269
Impacted Files Coverage Δ
cmd/skaffold/app/cmd/completion.go 0% <0%> (ø) ⬆️
pkg/skaffold/build/docker/docker.go 11.86% <0%> (-5.21%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e175e2...07e54e2. Read the comment docs.

@balopat balopat added the kokoro:run runs the kokoro jobs on a PR label Feb 22, 2019
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Feb 22, 2019
@balopat balopat added the kokoro:run runs the kokoro jobs on a PR label Feb 22, 2019
@balopat
Copy link
Contributor

balopat commented Feb 22, 2019

Thank you for fixing this!!!

Must I therefore include a proper attribution for the wrapper script?

I think it's fine just as a reference to put a comment "inspired by kubectl completion script" and a link

@balopat
Copy link
Contributor

balopat commented Feb 22, 2019

On second thought - reading the Apache license maybe actually adding their copyright notice as attribution is not a bad idea. So in the end we would have

  • skaffold copyright notice at the top as it is now
  • then a comment describing that this is a derivative work of the kubectl wrapper with link
  • then the copyright notice from the kubectl file

@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Feb 22, 2019
@corneliusweig
Copy link
Contributor Author

@balopat Ok, I added the attribution to kubernetes. Is the go package path ok as a link?

@balopat balopat merged commit 3039001 into GoogleContainerTools:master Feb 25, 2019
@corneliusweig corneliusweig deleted the fix-zsh-completion branch February 25, 2019 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants