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

Allow setting copyright header file for generated completions #40023

Merged
merged 1 commit into from Jan 20, 2017

Conversation

soltysh
Copy link
Contributor

@soltysh soltysh commented Jan 17, 2017

This PR allows downstream vendors (like openshift) to generate completions with custom header, similarly to other generated code.

@fabianofranz ptal
@kubernetes/sig-cli-misc fyi

@soltysh soltysh added release-note-none Denotes a PR that doesn't merit a release note. sig/cli Categorizes an issue or PR as relevant to SIG CLI. labels Jan 17, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 17, 2017
@@ -103,6 +104,7 @@ func NewCmdCompletion(f cmdutil.Factory, out io.Writer) *cobra.Command {
},
ValidArgs: shells,
}
cmd.Flags().String("boiler-plate", "", "The name of the copyright header file, defaults to hardcoded header value.")
Copy link
Contributor

Choose a reason for hiding this comment

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

a bit odd to have this parameter here. Would have expected some way to change the boilerplate per command such that downstream like oc can customize that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Odd because this feels like the user can choose the licence himself (I know the actual use-case to export completions and check them in for origin; but kubectl completion is also a user-facing command).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair argument, lemme see how can I address that.

@k8s-github-robot k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 17, 2017
@k8s-ci-robot
Copy link
Contributor

Jenkins verification failed for commit edbbe0a. Full PR test history. cc @soltysh

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 19, 2017
@soltysh
Copy link
Contributor Author

soltysh commented Jan 19, 2017

@sttts is this approach more to your liking?

@sttts
Copy link
Contributor

sttts commented Jan 19, 2017

@soltysh much better. lgtm

@fabianofranz
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 19, 2017
@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 39446, 40023, 36853)

@k8s-github-robot k8s-github-robot merged commit 71802d2 into kubernetes:master Jan 20, 2017
@soltysh soltysh deleted the completions_boilerplate branch January 20, 2017 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. sig/cli Categorizes an issue or PR as relevant to SIG CLI. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants