-
Notifications
You must be signed in to change notification settings - Fork 84
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
Script to automatically add licence headers #82
Conversation
/cc @ixdy @mikedanese |
verify/boilerplate/boilerplate.py
Outdated
@@ -1,6 +1,6 @@ | |||
#!/usr/bin/env python | |||
|
|||
# Copyright 2015 The Kubernetes Authors. | |||
# Copyright 2018 The Kubernetes Authors. |
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.
The year is not modified on updates. It reflects the year the file was added in.
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.
Sorry ... this came from testing the boilerplate code on the boilerplate code itself [1] :) Will revert that.
@@ -1,3 +1,5 @@ | |||
#!/usr/bin/env python |
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.
👍
verify/verify-boilerplate.sh
Outdated
|
||
# Copyright 2014 The Kubernetes Authors. | ||
# Copyright 2018 The Kubernetes Authors. |
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.
should remain as 2014 only, since that is the year the file was created
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.
[1]
verify/boilerplate/boilerplate.py
Outdated
"pkg/generated/bindata.go"] | ||
|
||
# list all the files that contain 'DO NOT EDIT', but are not generated | ||
default_skipped_not_generated = ['hack/build-ui.sh', 'hack/lib/swagger.sh', |
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 might be wrong here but my understanding was that repo-infra
is for all repositories under the kubernetes
org and not just k/k. Wouldn't this hard core it to k/k only?
(I understand that this was present even before but just curious :))
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.
As of now, we are not sure where repo-infra
is actually used. We'd like to see it being used everywhere, also in k/k
.
As we started off using the script from k/k
we kept the defaults as they were in the k/k
repo. However, we introduced an option to override those via a config file in other repos. An example of a config file can be found on this PR for this repo here.
Maybe we can add more (clearer) docs about exactly that. There is some documentation about that here.
Another related question is: Where should this config file be located. Right now we expect it to be in ${REPO_ROOT}/boilerplate.json
. If you think that is a too prominent location, we are happy to switch to e.g. ${REPO_ROOT}/.boilerplate.json
or something else -- just let us know.
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.
+1 to .boilerplate.json
instead of boilerplate.json
.
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.
and also I agree with @nikhita that having k/k defaults here seems weird.
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.
- We will remove the
k/k
defaults and come up with a set of defaults that are reasonable for a broad range of repos - We will switch to using
.boilerplate.json
for the config file
@@ -0,0 +1,16 @@ | |||
/* | |||
Copyright The Kubernetes Authors. |
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.
👍
To follow the same conventions as scripts in hack/, can we rename this to Also, I am a little confused about how repo-infra is being used for k/k right now. k/k has it's own boilerplate verification: https://github.com/kubernetes/kubernetes/blob/master/hack/verify-boilerplate.sh and https://github.com/kubernetes/kubernetes/tree/master/hack/boilerplate. |
|
No strong opinions. I'm not aware of the history between repo-infra and k/k. I'll defer to @ixdy @spiffxp @cblecker on that one. |
…stead. Signed-off-by: Jintao Zhang <zhangjintao9020@gmail.com>
with open(conf_path) as json_data_file: | ||
return json.load(json_data_file) | ||
except ValueError: | ||
raise |
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.
👍
cc @justaugustus @dims @swinslow This relates to our licensing efforts in Kubernetes. |
Is there anything we can do to get this merged? Or even more importantly: is there a way to track which repos actually use this repo and therefore assess if it's even relevant? |
I believe the verify script for license headers is not used anywhere currently. k/k has its own verify script but the plan is to start using this repo in the future. |
Right, I don't have a good answer for how we can easily vendor scripts from repo-infra and keep them up to date; just copying the scripts from repo-infra into the various other repos is not great. Anyone have any good suggestions? |
What if this repo had a script which would update itself via I'd be up for experimenting with that in |
... automation should really be removing the needs-rebase label automatically on push |
/lifecycle stale Any chance you're going to come back to this? It looks cool and super useful! |
@fejta I think all the issues and comments are addressed. So I think it is done from our perspective. |
Cool! Meaning you're waiting on a review? /remove-lifecycle stale |
Yes, that'd be great. |
Bumping this. Looks like this needs more reviewers (I'm way out of touch with python) |
Can we merge and iterate here? We require donated repos to have this boilerplate header for all their files. Adding the header to each file in a repo is extremely cumbersome, this script would be immensely helpful in such cases. |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
/remove-lifecycle stale |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
@mariantalla: PR needs rebase. 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. |
Rotten issues close after 30d of inactivity. Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
@fejta-bot: Closed this PR. In response to this:
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. |
This PR adds a
ensure-boilerplate.sh
script; when run, it will add copyright/licence headers to applicable files.Where a valid licence header is already present, the script will not update or otherwise change it. Similarly for files that it is configured to
skip
.As discussed with sig-contributor-experience this is copied from pivotal-k8s/kubernetes-contrib-tools - we'll deprecate the latter once this PR is merged.
/cc @hoegaarden