-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Remove unnecessary copyright remarks in our code #12613
Conversation
@@ -230,7 +230,7 @@ func NewCmdCompletion(fullName string, f *clientcmd.Factory, out io.Writer) *cob | |||
cmdHelpName = "openshift" | |||
} | |||
|
|||
cmd := kcmd.NewCmdCompletion(f, out) | |||
cmd := kcmd.NewCmdCompletion(f, out, "\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why \n
and not just the empty string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because empty triggers to use a default boilerplate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that's annoying
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How else would you think of doing it upstream?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think what you did upstream is good and the right way to do it. For aesthetics, I would prefer to avoid a newline but I guess it's not easily doable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially I went with a space, but that looked bad, so I fall back to newline, which looks much better in the generated file.
@ncdc any other concerns? |
LGTM |
[merge] |
[Test]ing while waiting on the merge queue |
Evaluated for origin test up to 6d7beab |
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13185/) (Base Commit: 6822a9b) |
Flake #10773 [merge] |
Evaluated for origin merge up to 6d7beab |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13213/) (Base Commit: b0f2c58) (Image: devenv-rhel7_5757) |
@ncdc one of post-rebase task, ptal