Add required phrase markers to CC license rules#3644
Add required phrase markers to CC license rules#3644pombredanne merged 3 commits intoaboutcode-org:developfrom
Conversation
AyanSinhaMahapatra
left a comment
There was a problem hiding this comment.
thanks @dotarjun
We have to make sure here the detection issue is fixed completely.
When you're fixing these issues do attach scan results here:
custom-content.html.txt.json
scancode -l --license-text --license-diagnostics --license-text-diagnostics --license-references --todo custom-content.html.txt --json-pp custom-content.html.txt.json
- Here we still have a
cc-by-sa-4.0detection which is incorrect, matched to https://github.com/nexB/scancode-toolkit/tree/develop/src/licensedcode/data/rules/cc-by-sa-4.0_44.RULE so we have to add a required phrase there too.
This is a process of adding a change (be it a new rule or adding a required phrase to an old rule), reindex withscancode-reindex-licensesand scan again to check that the detection issue is fixed and the results are correct. - We can also improve the commit message and content.
bug: add double curly braces->Add required phrases to rulesand add the lines:
Reference: https://github.com/nexB/scancode-toolkit/issues/3615
Reported-by: @camillem
|
By following that command, I was running into this error: So I ran this instead. This command did make some changes to But then I am unable to figure out what is going on. For reference, I am using arch linux and my dev env is setup as per https://scancode-toolkit.readthedocs.io/en/latest/getting-started/install.html#installation-from-source-code-git-clone. I have it configured with Concerning the git commit message, |
Reference: aboutcode-org#3615 Reported-by: @camillem Signed-off-by: dotarjun <[email protected]>
e764391 to
dc7ff50
Compare
|
This is looking great! |
That's because you don't have the file there.
By doing this you're scanning the result file and outputting the result there again, why would you want to do that? Basically we want to reindex the licenses with your change here in the PR, then scan the test file again to see that there are no more similar bugs present for the same case (by scanning with the commands above, you will find another similar bug, and you have to add another required phrase to another rule to fix this). Or as @pombredanne mentioned above you can add a test and make sure the detections are correct, and there you'll se the issue. And then:
|
|
Thank you for the guidance. I totally missed it as I have my exams going on. I'll do it tomorrow after my exam. Apologies for taking such a long time. |
I think cc-by-sa-4.0 should not be detected. Am I on the right track? |
Yes! Now we have to update |
|
I've been trying to figure out how to do that but I can't understand why that is happening.
This makes no sense as in Everything is correct in my eyes already |
Signed-off-by: dotarjun <[email protected]>
|
You should add a test under tests/ IMHO |
|
@AyanSinhaMahapatra do you want to wait for a test, or merge this as-is? |
|
I am merging, but there are not much changes in the PR at all since there have been many updates in the meantime! |

Fixes #3615
Tasks
Run tests locally to check for errors.