feat: add license compliance check tool#66
feat: add license compliance check tool#66pbredenberg wants to merge 5 commits intosilvermine:masterfrom
Conversation
scripts/check-license-compliance.js
Outdated
| /** | ||
| * Async wrapper around `licence-checker`'s init function. | ||
| * @param options | ||
| */ | ||
| const initAsync = async (options) => { | ||
| return new Promise((resolve, reject) => { | ||
| init(options, (err, packages) => { | ||
| if (err) { | ||
| reject(err); | ||
| } | ||
|
|
||
| resolve(packages); | ||
| }); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Would something like this work?
const { init } = require('license-checker'),
util = require('util'),
initLicenseChecker = util.promisify(init);
pbredenberg
force-pushed
the
pb/feat-license-checker
branch
from
406e881 to
c030afd
Compare
pbredenberg
force-pushed
the
pb/feat-license-checker
branch
from
c030afd to
acfc3d5
Compare
.license-checker.cjs
Outdated
|
|
||
| module.exports = { | ||
|
|
||
| allowList: [ |
There was a problem hiding this comment.
What about this?
| allowList: [ | |
| permittedLicenses: [ |
scripts/check-license-compliance.js
Outdated
|
|
||
| (async () => { | ||
| const projectDirectoryPath = await packageDirectory(), | ||
| projectConfigPath = `${projectDirectoryPath}/.license-checker.cjs`, |
There was a problem hiding this comment.
Any particular reason this is .cjs and not just .js?
There was a problem hiding this comment.
Haha, no. I have no idea why I did that.
scripts/check-license-compliance.js
Outdated
| console.info(`This project has ${Object.values(licenseRecord).length} licenses.`); | ||
|
|
||
| unsupportedLicenses = Object.values(licenseRecord).filter((record) => { | ||
| return !ALLOWED_LICENSES.includes(record.license) && !allowList.includes(record.license); |
There was a problem hiding this comment.
If a project supplies a list of permitted licenses, should we keep checking the default license list?
There was a problem hiding this comment.
I was imagining the licenses passed in would be in addition to default set declared inside the tool.
scripts/check-license-compliance.js
Outdated
| moduleInfoReport[moduleName].licenses.forEach((license) => { | ||
| licenseRecord[moduleName] = { | ||
| module: moduleName, | ||
| license: license, | ||
| ...moduleInfoReport[moduleName], | ||
| }; | ||
| }); |
There was a problem hiding this comment.
This only keeps the last license. Would it be better to have a list of all the licenses and see if any are in the list of permitted licenses?
There was a problem hiding this comment.
You're right! I neglected to revisit this block of code. Here's the situation: in the many projects I've tested it in, I've never seen const licenses = moduleInfoReport[moduleName].licenses; evaluate to an array.
That's probably because the library is handling an edge case due to what the NPM docs say here regarding license data which they refer to as invalid.
In short, I don't think this block is actually needed from looking at the source linked above.
There was a problem hiding this comment.
I'm now simply taking whatever value it sends. Let me know if you think we should still support this case.
scripts/check-license-compliance.js
Outdated
| .forEach((moduleName) => { | ||
| const licenses = moduleInfoReport[moduleName].licenses; | ||
|
|
||
| if (!licenses) { |
There was a problem hiding this comment.
If there are no licenses, should we assume that the package is UNLICENSED (i.e. the package cannot be used by anyone but the copyright holder)?
There was a problem hiding this comment.
Sure, good point!
|
|
||
| Object | ||
| .keys(moduleInfoReport) | ||
| .forEach((moduleName) => { |
There was a problem hiding this comment.
We'll also need a way to note what packages are permitted regardless of their license. Perhaps a config option like permittedPackages?
There was a problem hiding this comment.
Ok, sounds good.
There was a problem hiding this comment.
@onebytegone this hath been added, my liege
pbredenberg
force-pushed
the
pb/feat-license-checker
branch
from
acfc3d5 to
ccc6ebb
Compare
pbredenberg
force-pushed
the
pb/feat-license-checker
branch
from
ccc6ebb to
93f1035
Compare
2 participants
No description provided.