Skip to content

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

Merged
balopat merged 2 commits intoGoogleContainerTools:masterfrom
corneliusweig:fix-zsh-completion
Feb 25, 2019
Merged

completion: add wrapper code to transform bash to zsh completion#1685
balopat merged 2 commits intoGoogleContainerTools:masterfrom
corneliusweig:fix-zsh-completion

Conversation

@corneliusweig
Copy link
Copy Markdown
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 <cornelius.weig@tngtech.com>
@codecov-io
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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

Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Feb 22, 2019
@corneliusweig
Copy link
Copy Markdown
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