-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add copyright #391
Add copyright #391
Conversation
tools/gen-release-vote.py
Outdated
@@ -94,7 +96,7 @@ def gitHashes(components): | |||
return '\n'.join(list(s)) | |||
|
|||
def rcverify(components, version): | |||
s = map(lambda r: "./rcverify.sh %s '%s' %s %s" % (r.id, r.name, version.v, version.rc), components) | |||
s = map(lambda r: "./rcverify.sh %s '%s' %s %s %s" % (r.id, r.name, version.v, version.rc, r.copyright), components) |
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.
It generates the usage like this:
Usage:
curl -s "https://gitbox.apache.org/repos/asf?p=openwhisk-release.git;a=blob_plain;f=tools/rcverify.sh;hb=12cc460" -o rcverify.sh
chmod +x rcverify.sh
./rcverify.sh openwhisk-vscode-extension 'OpenWhisk VSCode Extension' 1.1.0 rc1 2020-2021
tools/local_verify.sh
Outdated
if [ ! -n "$COPYRIGHT" ] | ||
then | ||
echo "no copyright year provided, fallback to the default: 2016-2020" | ||
COPYRIGHT="2016-2020" |
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.
If no copyright year is provided it will be used.
tools/rcverify.sh
Outdated
@@ -69,7 +72,7 @@ KEYS=$RC-$V-KEYS | |||
|
|||
NOTICE=$(cat << END | |||
Apache $DESCRIPTION | |||
Copyright 2016-2021 The Apache Software Foundation | |||
Copyright $COPYRIGHT The Apache Software Foundation |
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.
This needs to be configurable as IDE plugins use 2020-2021
in the NOTICE.txt.
https://github.com/apache/openwhisk-intellij-plugin/blob/master/NOTICE.txt#L2
https://github.com/apache/openwhisk-vscode-extension/blob/master/NOTICE.txt#L2
What we really should be scanning for is a copyright of the form:
Where NN are any number that is <= the end year (21). It's important that we check for the end year being the same as the current year; we don't want to make that configurable |
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.
Having the end year of the copyright be configurable is not desirable; will lead to failure to update the end year when making a release
@dgrove-oss Now only the starting year is configurable. |
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.
Thanks; this will work.
It would be nice if we used a regex to validate the NOTICE file so we could avoid specifying both the r.name and r.copyright, but that's more work than needs to be done to unblock the ide-plugin releases.
@@ -183,6 +177,21 @@ function analyzeKeyImport() { | |||
fi | |||
} | |||
|
|||
function validateNotice() { |
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 have tested all NOTICE.txt
files in all repos, and confirmed it works as expected.
But it would be great if anyone cross-checks again.
Some of them require changes so I opened a couple of corresponding PRs.
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.
Nice! Thank you @style95
NOTICE=$(cat << END | ||
Apache $DESCRIPTION | ||
Copyright 2016-2021 The Apache Software Foundation | ||
NOTICE_REGEX='^Apache .+ |
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.
This is a positive change - thanks for improving the checks.
Removing the component description from the check could lead to an error where someone copies the notice file from one repo and forgets to update the name. Will have to be diligent about this.
It's nice to have one fewer argument to rcverify.
This it to make the copyright year configurable.