-
Notifications
You must be signed in to change notification settings - Fork 35
License checker #1028
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
License checker #1028
Conversation
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 second commit, with just a permission update should be squashed.
Also, at best the first commit would contain orignal script, and the second commit would contain changes for this repo (incl. dates update)
There is an error in both new jobs: |
I think we can ignore it. I cannot see any reason of that. |
Me neither. CI works as supposed, locally I cannot reproduce - LGTM |
check_license/check_headers.sh
Outdated
CURRENT_COMMIT=$($GIT --no-pager log --pretty=%H -1) | ||
MERGE_BASE=$($GIT merge-base HEAD origin/main 2>/dev/null) | ||
[ -z $MERGE_BASE ] && \ | ||
MERGE_BASE=$($GIT --no-pager log --pretty="%cN:%H" | grep GitHub | head -n1 | cut -d: -f2) |
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.
Generally this fallback causes error because of SIGPIPE signal. Redirecting stderr solves the problem (like in 72 line)
--pretty="%cN:%H" 2>/dev/null | grep "GitHub" 2>/dev/null | head -n 1 | cut -d: -f2)
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.
Thank you for your help, your idea solved the problem 🥇
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.
All 3 commits:
https://github.com/oneapi-src/unified-memory-framework/pull/1028/commits
should be squashed into one (2nd and 3rd commit are fixes of the 1st one)
@ldorau, I discussed this with @rbanka1, I believe it's ok. The first one (more or less) introduces new script (based on pmemstream), the second one extends its capabilities, and the third one fixes the issue, which existed also in the original script... do you think it's not ok to have all these 3 commits here...? |
@lukaszstolarczuk So why not squash commit 3rd with 2n? |
we can do that :) - please remember @rbanka1 to update commit msg with all changes |
d8550c5
to
007a4cd
Compare
based on https://github.com/pmem/pmemstream/tree/master/utils/check_license Changes for this repository: - update dates - file exceptions
… fixes Changes for this commit: - adding a condition to check the validity of the starting date - broken pipe fix - updating date fix
based on https://github.com/pmem/pmemstream/tree/master/utils/check_license
Description
Added license checker and exception file.
Checklist