-
Notifications
You must be signed in to change notification settings - Fork 35
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
Licensed under "Custom: https://github.com..." #63
Comments
Okay, so I managed to solve it. My root {
"name": "mypkg",
"version": "1.0.0",
"description": "Root npm project, only meant to control workspaces.",
"workspaces": [
"packages/mypkg"
],
"scripts": {
"blah blah": "random command here"
},
"devDependencies": {
"openapi-typescript-codegen": "^0.23.0"
}
} And all I had to do was insert the {
"name": "mypkg",
"version": "1.0.0",
"description": "Root npm project, only meant to control workspaces.",
+ "license": "MYLICENSE-HERE",
"workspaces": [
"packages/mypkg"
],
"scripts": {
"blah blah": "random command here"
},
"devDependencies": {
"openapi-typescript-codegen": "^0.23.0"
}
} |
However, I'm now noticing that the checker is unable to check for dependencies underneath this npm workspace. |
Hi @fredrikaverpil , thanks for informing me about this. I've never heard of npm workspaces before and I will have a more detailed look into the documentation - thank you for linking it in your comment. Indeed, the I do not promise anything here, but I plan on dealing with it the next days. Also, of course, feel free to create a pull request on your own if you feel like it. I always appreciate any kind of input - be it a pull request or just a question or note. Cheers, Roman. |
@RSeidelsohn there's an open issue about this here: #36 |
Hi, We are also seeing this occur an increasing amount of times in various of our repos as we've rolled out license checking in more of our JS repos. Another example Reproduction:
{
"dependencies": {
"cycle": "^1.0.3"
}
}
Output
In the published version of We've seen similar errors where some other package which has a proprietary license suddenly get parsed as having a |
@RSeidelsohn After some debugging and digging in the code when there is a custom license specified, for example a URL in package.json license-checker-rseidelsohn/lib/index.js Lines 195 to 204 in d95e43e
Which will then override the already correctly detected license from the license field. |
#71 shows a failing test for this |
I think there are two parts to this. license-checker-rseidelsohn/lib/index.js Lines 162 to 164 in d95e43e
Then somehow deciding when to check these additional files or not if there already is a custom URL detected? |
Sorry guys, this is a flaw I have to fix. I'll dig into this code next - unfortunately, this code was neither created by me, nor did I get a hand-over. So I have to try to understand what's going on here and why so and only then I'll be able to fix it. Should anyone of you or someone else feel the urge to provide a fix: PR's are much appreciated! |
Hey @Flydiverny and @fredrikaverpil , I am finally on it! I had to refactor quite some of the still old logic, which helped me understand more and even find and fix another bug. I'm now quite near, but still, please bear with me. Holidays are approaching and I guess I won't do much during that time. Cheers, Roman. |
Thanks a lot for having a look at it @RSeidelsohn! Enjoy the holidays :) 🐣 |
Hi @Flydiverny and @fredrikaverpil , a short update: I have worked a lot on the code during my vacation and just released a bugfix version 4.2.1, which at least pleases the new test you added, @Flydiverny , but still does not respect npm workspaces (I guess - I did not test this yet). |
Ok, the whole functionality for automatically finding a license in a README file is pretty doomed. Currently, the license-checker just looks for the first URL in the README, which leads to the situation you mention. This doesn't make any sense and I have to think of a way to improve on this. UpdateI released a kind of a fix for it in version 4.2.4 - URLs in READMEs will now only be taken as a last resort if the READMEs at least include the term "license". This fixes already a lot of false hits. |
I don't know if this helps in the long run, but I didn't like how the --files output So I wrote two regexes to extract everything under a license markdown header - one for
e.g.
or
and
They only extract the text until the next header in the README, or the end of file. They are in .NET flavour but probably work for other regex flavours, just strip out the ?<license> and ?<header> tags, which is .NET specific syntax.
An example of 1 at regex101 and 2 at regex101
|
Hi,
Have you ever seen something like this after running the checker in GitHub Actions?
I can provide more info but figured I'd start like this. My project is using npm workspaces which might be related to this...
The text was updated successfully, but these errors were encountered: