-
Notifications
You must be signed in to change notification settings - Fork 8
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 license check #861
base: main
Are you sure you want to change the base?
add license check #861
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #861 +/- ##
=======================================
Coverage 95.58% 95.58%
=======================================
Files 125 125
Lines 5419 5419
=======================================
Hits 5180 5180
Misses 239 239 ☔ View full report in Codecov by Sentry. |
23fc5a4
to
5326bfe
Compare
This is only run on release- shouldn't we be checking during the PR to add the dependency? |
good question @DiamondJoseph I'd say yes the footwork should be done beforehand. The license check in the release is more just for a sanity check, it does not need to happen at the early stage. two layers > one |
5326bfe
to
d74761c
Compare
What I mean @stan-dot is that this action should be run during the PR, not that it should be part of the review process. If we add a dependency that is not licensed, then we make use of it it'll be difficult to extract after the fact. I'd expect this PR to have triggered a new check so that we could see if we are not already non-compliant. |
fair enough @DiamondJoseph , where exactly do you consider it is best to run it then? before the lint? after the test? |
It's independent of those, it's just another workflow to be run. Make it a new |
This doesn't appear to be working, there is no license.json uploaded as an artifact from the job... |
Fixes #825