-
Notifications
You must be signed in to change notification settings - Fork 780
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
ci: add license lint wf for cncf approved licenses #2461
ci: add license lint wf for cncf approved licenses #2461
Conversation
e9d030a
to
e08e622
Compare
Codecov ReportBase: 53.95% // Head: 53.90% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #2461 +/- ##
==========================================
- Coverage 53.95% 53.90% -0.05%
==========================================
Files 116 116
Lines 10286 10286
==========================================
- Hits 5550 5545 -5
- Misses 4311 4315 +4
- Partials 425 426 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
e08e622
to
6c3983d
Compare
6c3983d
to
158461d
Compare
hey @sozercan & @maxsmythe thanks both for offering feedback on the initial approach. After this comment that Max made I realized that the
At present, if a package under allow list changes from an approved license to an unapproved license, the tool wouldn't detect that. code ref While I am not too sure how often this would happen, that scenario, combined with some other papercuts of the tool have convinced me to explore a different solution for what actually does the license linting. |
Starting 4e73471 I am introducing a script from the k8s repo: verify-licenses and its dependencies. I also added a few commits to taylor it to the g8r repo. Some of the env vars and such will remain. |
Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
8ee2596
to
60795a7
Compare
Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
60795a7
to
46e88a8
Compare
Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
alright folks, I think this PR has all the feedback from our weekly chat @ritazh @sozercan @maxsmythe |
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.
LGTM. since action didn't run on this PR, did you test this manually?
re testing, there's 3 avenues:
|
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.
LGTM
…#2461) * make yml file for current g8r licenses Signed-off-by: Alex Pana <[email protected]> * add gh wf for license-lint Signed-off-by: Alex Pana <[email protected]> * cncf aligned config for lgk Signed-off-by: Alex Pana <[email protected]> * cherry pick script at 124fd62ad25 Signed-off-by: Alex Pana <[email protected]> * replace some k8s references, fix path Signed-off-by: Alex Pana <[email protected]> * swap out license linter Signed-off-by: Alex Pana <[email protected]> * delete license-lint config Signed-off-by: Alex Pana <[email protected]> * designer commits: specify CF url, add exception, include tests Signed-off-by: Alex Pana <[email protected]> * add wf paths Signed-off-by: Alex Pana <[email protected]> * add readme Signed-off-by: Alex Pana <[email protected]> * remove dependencies Signed-off-by: Alex Pana <[email protected]> Signed-off-by: Alex Pana <[email protected]>
Fixes #2404
reviewer notes
Initially, i wanted to use
license-lint
but after an issue wallowlisted
modules where the underlying license can change without detection, I decided to go with Max's original suggestion in the issue: https://github.com/kubernetes/kubernetes/blob/master/hack/verify-licenses.sh . This is a script that I am copying over w a number of minimal changes to make it work for us.act
! https://github.com/nektos/act