Skip to content

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

Merged
merged 2 commits into from
Jan 14, 2025
Merged

License checker #1028

merged 2 commits into from
Jan 14, 2025

Conversation

rbanka1
Copy link
Contributor

@rbanka1 rbanka1 commented Jan 8, 2025

based on https://github.com/pmem/pmemstream/tree/master/utils/check_license

Description

Added license checker and exception file.

Checklist

  • Code compiles without errors locally
  • All tests pass locally
  • CI workflows execute properly
  • CI workflows, not executed per PR (e.g. Nightly), execute properly
  • New tests added, especially if they will fail without my changes
  • Added/extended example(s) to cover this functionality
  • Extended the README/documentation
  • All newly added source files have a license
  • All newly added source files are referenced in CMake files
  • Logger (with debug/info/... messages) is used

Copy link
Contributor

@lukaszstolarczuk lukaszstolarczuk left a 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)

@lukaszstolarczuk lukaszstolarczuk marked this pull request as ready for review January 9, 2025 10:26
@lukaszstolarczuk lukaszstolarczuk requested a review from a team as a code owner January 9, 2025 10:26
@PatKamin
Copy link
Contributor

PatKamin commented Jan 9, 2025

There is an error in both new jobs: grep: write error: Broken pipe

@lukaszstolarczuk lukaszstolarczuk marked this pull request as draft January 9, 2025 10:47
@lukaszstolarczuk lukaszstolarczuk mentioned this pull request Jan 9, 2025
10 tasks
@lukaszstolarczuk lukaszstolarczuk marked this pull request as ready for review January 9, 2025 12:21
@ldorau
Copy link
Contributor

ldorau commented Jan 9, 2025

There is an error in both new jobs: grep: write error: Broken pipe

I think we can ignore it. I cannot see any reason of that.

@PatKamin
Copy link
Contributor

PatKamin commented Jan 9, 2025

There is an error in both new jobs: grep: write error: Broken pipe

I think we can ignore it. I cannot see any reason of that.

Me neither. CI works as supposed, locally I cannot reproduce - LGTM

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)
Copy link
Contributor

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)

Copy link
Contributor Author

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 🥇

Copy link
Contributor

@ldorau ldorau left a 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)

@lukaszstolarczuk
Copy link
Contributor

lukaszstolarczuk commented Jan 10, 2025

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...?

@ldorau
Copy link
Contributor

ldorau commented Jan 10, 2025

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?

@lukaszstolarczuk
Copy link
Contributor

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

@rbanka1 rbanka1 force-pushed the licenseNew branch 2 times, most recently from d8550c5 to 007a4cd Compare January 13, 2025 10:11
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
@lukaszstolarczuk lukaszstolarczuk merged commit bf8d0d8 into oneapi-src:main Jan 14, 2025
78 checks passed
@rbanka1 rbanka1 deleted the licenseNew branch February 18, 2025 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants