Scripts: Refactor to extract license logic #66179
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sirreal
added
[Type] Task
sirreal
requested review from
gziolo,
ntwb,
nerrad,
ajitbohra and
ryanwelcher
as code owners
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: 0 B Total Size: 1.77 MB ℹ️ View Unchanged
|
|
Flaky tests detected in 17d3280. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11382438174
|
gziolo
approved these changes
Oct 17, 2024
There was a problem hiding this comment.
Everything looks good. I see that some types and better descriptions were added. We could iterate further on it, as there is still room for improvement. Yes, you opened a Pandora's box. However, not a blocker for this PR in the spirit of Boy Scout Rule:
Leave your code better than you found it.
gziolo
added
[Type] Code Quality
This license appears on some npm packages. It intended to be highly permissive. The website states that it should be GPLv2 compatible: > Is the model license compatible with GPL? > The Council doesn’t see any reason why software licensed under the > Blue Oak Model License 1.0.0 can’t be used, combined, and distributed > with software under GPLv2, LGPLv2.1, GPLv3, LGPLv3, or AGPLv3. https://blueoakcouncil.org/license-faq
sirreal
force-pushed
the
update/scripts-check-licenses-extract-logic
branch
from
935cd7e to
17d3280
Compare
karthick-murugan
pushed a commit
to karthick-murugan/gutenberg
that referenced
this pull request
Nov 13, 2024
Refactor some of the check-license logic into a reusable module. The check-license script continues to act as the CLI, while another module contains the license checking logic. This is helpful for work migrating to npm workspaces: WordPress#65681 The license checking logic is required, but the implementation will be different to account for npm workspaces in Gutenberg. --- Co-authored-by: sirreal <[email protected]> Co-authored-by: gziolo <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Part of #65681, this refactors some of the
check-licenselogic in wp-scripts into a reusable module. Thecheck-licensescript continues to act as the CLI, while another module contains the license checking logic.Why?
#65681 will reuse this logic in a way that's compatible with npm workspaces in Gutenberg. It cannot use the same check-licenses CLI logic that depends on repository dependencies and devDependencies.
How?
This moves code around and adjusts some function signatures to pass arguments around.
Testing Instructions
Unit tests are included.