Skip to content

contrib/completion: remove "compose on kubernetes" flags zsh completion - #3439

Merged
thaJeztah merged 2 commits into
docker:masterfrom
thaJeztah:drop_kube_completion
May 2, 2022
Merged

contrib/completion: remove "compose on kubernetes" flags zsh completion#3439
thaJeztah merged 2 commits into
docker:masterfrom
thaJeztah:drop_kube_completion

Conversation

@thaJeztah

@thaJeztahthaJeztah commented Feb 24, 2022

Copy link
Copy Markdown
Member

@codecov-commenter

codecov-commenter commented Feb 24, 2022•
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #3439 (2d74be8) into master (9f8ccfc) will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@## master #3439 +/- ##
=======================================
Coverage 59.48% 59.48% =======================================
Files 287 287 Lines 24182 24182 =======================================
Hits 14385 14385 Misses 8931 8931 Partials 866 866

albers
albers reviewed Feb 27, 2022
View reviewed changes

@albersalbers left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Bash completion LGTM. I found some nits regarding consistency.

Keeping the $options variables instead of inlining them probably improves readybility.
But the existing code only uses such variables where option lists are constructed incrementally, so for consistency I recommend to remove them.

Comment threadcontrib/completion/bash/docker Outdated
Comment threadcontrib/completion/bash/docker Outdated
Comment threadcontrib/completion/bash/docker Outdated
Comment threadcontrib/completion/bash/docker Outdated
Comment threadcontrib/completion/bash/docker Outdated
Comment threadcontrib/completion/bash/docker Outdated
Comment threadcontrib/completion/bash/docker Outdated
@thaJeztahthaJeztah mentioned this pull request Apr 30, 2022
Merged
@thaJeztahthaJeztah reopened this May 1, 2022
@thaJeztah
contrib/completion: remove unused options variable
ea0ae30
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
contrib/completion: remove orchestrator flags from zsh completion
2d74be8
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
thaJeztahforce-pushed the drop_kube_completion branch from 7251b17 to 2d74be8CompareMay 1, 2022 12:04
@thaJeztahthaJeztah changed the title contrib/completion: remove "compose on kubernetes" flags from completion scriptscontrib/completion: remove "compose on kubernetes" flags zsh completionMay 1, 2022
thaJeztah
thaJeztah commented May 1, 2022
View reviewed changes
Comment threadcontrib/completion/bash/docker
@thaJeztahthaJeztah added this to the 22.04.0 milestone May 1, 2022
@thaJeztah

thaJeztah commented May 1, 2022

Copy link
Copy Markdown
MemberAuthor

@albers rebased this one, and removed the bash completion changes (spotted one variable we missed); ptal

albers
albers approved these changes May 2, 2022
View reviewed changes

@albersalbers left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, though I have to admit that I'm not an expert on zsh completion.

@thaJeztah

thaJeztah commented May 2, 2022

Copy link
Copy Markdown
MemberAuthor

hehe, neither am I, but I think removing those lines should be "good".

Thanks for looking; let me get this one in!

@thaJeztah
thaJeztah merged commit ba6f260 into docker:masterMay 2, 2022
@thaJeztah
thaJeztah deleted the drop_kube_completion branch May 2, 2022 11:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@albersalbersalbers approved these changes

Assignees

No one assigned

Labels

area/completionarea/kubernetesarea/stackimpact/deprecationstatus/2-code-review

Projects

None yet

Milestone

23.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@thaJeztah@codecov-commenter@albers