-
-
Notifications
You must be signed in to change notification settings - Fork 553
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 required phrase markers to CC license rules #3644
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.0
detection 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-licenses
and 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 rules
and add the lines:
Reference: https://github.com/nexB/scancode-toolkit/issues/3615
Reported-by: @camillem
See also https://aboutcode.readthedocs.io/en/fix-rtd-pages/contributing/writing_good_commit_messages.html
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? |
Fixes #3615
Tasks
Run tests locally to check for errors.