Skip to content

Report unknown licenses separately. - #2578

Closed
akugarg wants to merge 4 commits into
aboutcode-org:developfrom
akugarg:Report_licenses_separately
Closed

Report unknown licenses separately.#2578
akugarg wants to merge 4 commits into
aboutcode-org:developfrom
akugarg:Report_licenses_separately

Conversation

@akugarg

@akugarg akugarg commented Jun 28, 2021

Copy link
Copy Markdown
Contributor

Signed-off-by: akugarg akanksha.garg2k@gmail.com

Fixes #2574

Tasks

  • Reviewed contribution guidelines
  • PR is descriptively titled 📑 and links the original issue above 🔗
  • Tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR
    Run tests locally to check for errors.
  • Commits are in uniquely-named feature branch and has no merge conflicts 📁

AyanSinhaMahapatra
AyanSinhaMahapatra requested changes Jun 28, 2021
View reviewed changes

@AyanSinhaMahapatra AyanSinhaMahapatra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, we have to create a resource attribute like this here, and you have to return unknown_licenses here.

Comment thread src/scancode/api.py Outdated

detected_licenses.extend(
_licenses_data_from_match(
if "unknown" in match.rule.license_expression:

@AyanSinhaMahapatra AyanSinhaMahapatra Jun 28, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

matches will have a is_unknown flag you added :P Use that instead of checking the license_eexpression.

@akugarg akugarg Jun 29, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah! Actually that PR is not yet merged therefore changes were not reflected in this branch which was giving errors, that's why I have used this .

pombredanne
pombredanne reviewed Jun 29, 2021
View reviewed changes
Comment thread src/scancode/api.py Outdated
@akugarg
Create a separate section for unknown licenses
5851f1f 
Signed-off-by: akugarg <akanksha.garg2k@gmail.com>
@akugarg
akugarg force-pushed the Report_licenses_separately branch from 9222562 to 5851f1f Compare June 30, 2021 05:28
AyanSinhaMahapatra
AyanSinhaMahapatra requested changes Jun 30, 2021
View reviewed changes

@AyanSinhaMahapatra AyanSinhaMahapatra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should fix the tests next.

Comment thread src/scancode/api.py
Comment thread src/scancode/api.py
Comment thread src/licensedcode/plugin_license.py Outdated
Comment thread src/scancode/api.py
@akugarg
Create a separate section for unknown licenses
decb959 
Signed-off-by: akugarg <akanksha.garg2k@gmail.com>
@akugarg
akugarg force-pushed the Report_licenses_separately branch 2 times, most recently from 9e91a14 to d514cbb Compare July 3, 2021 12:07
@akugarg
Update test cases upto expectation
ccf9de4 
Signed-off-by: akugarg <akanksha.garg2k@gmail.com>
@akugarg
akugarg force-pushed the Report_licenses_separately branch from d514cbb to ccf9de4 Compare July 5, 2021 04:45
@AyanSinhaMahapatra

AyanSinhaMahapatra commented Jul 6, 2021

Copy link
Copy Markdown
Member

Some tests are failing in summarycode/plugin_consolidate.py. See here. Another normalized_unknown_license_expression should be saved to each child, and then added to Consolidation.other_license_expression. And related test expectations fixed.

@akugarg
akugarg force-pushed the Report_licenses_separately branch from b26a578 to 412ea31 Compare July 7, 2021 06:58
AyanSinhaMahapatra
AyanSinhaMahapatra reviewed Jul 7, 2021
View reviewed changes
Comment thread src/summarycode/plugin_consolidate.py Outdated
AyanSinhaMahapatra
AyanSinhaMahapatra requested changes Jul 7, 2021
View reviewed changes
Comment thread src/summarycode/plugin_consolidate.py Outdated
@akugarg
akugarg force-pushed the Report_licenses_separately branch from 412ea31 to 908e8a4 Compare July 7, 2021 07:59
AyanSinhaMahapatra
AyanSinhaMahapatra reviewed Jul 7, 2021
View reviewed changes
Comment thread src/summarycode/plugin_consolidate.py Outdated

c = Consolidation(
core_license_expression=combine_expressions(license_expressions),
other_license_expression = combine_expressions(unknown_expressions),

@AyanSinhaMahapatra AyanSinhaMahapatra Jul 7, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No spaces should be present before and after = in this case.

pombredanne
pombredanne reviewed Jul 11, 2021
View reviewed changes
Comment thread src/scancode/api.py Outdated
)


if "unknown" in match.rule.license_expression: #TODO: use is_unknown flag instead

@pombredanne pombredanne Jul 11, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re #TODO: use is_unknown flag instead when would this happen?

@AyanSinhaMahapatra AyanSinhaMahapatra Jul 12, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pombredanne after #2548 is approved and merged, then this could be rebased/merged and the is_unknown flags introduced there can be used.

pombredanne
pombredanne requested changes Jul 11, 2021
View reviewed changes

@pombredanne pombredanne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. See my comments inline.

Comment thread src/scancode/api.py Outdated
Comment thread tests/cluecode/data/plugin_filter_clues/filtered-expected2.json Outdated
Comment thread tests/cluecode/data/plugin_filter_clues/filtered-expected3.json Outdated
"mime_type": "text/plain",
"file_type": "ASCII text",
"programming_language": null,
"programming_language": "verilog",

@pombredanne pombredanne Jul 11, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is weird... how is this related to the current PR?

Comment thread tests/formattedcode/data/json/tree/expected.json
@akugarg
akugarg force-pushed the Report_licenses_separately branch from 33634cf to 7c22012 Compare July 11, 2021 19:36
@akugarg
Update test cases upto expectation
8f93fda 
Signed-off-by: akugarg <akanksha.garg2k@gmail.com>
@akugarg
akugarg force-pushed the Report_licenses_separately branch from 7c22012 to 8f93fda Compare July 12, 2021 06:02
@pombredanne

pombredanne commented Aug 6, 2021

Copy link
Copy Markdown
Member

@akugarg @AyanSinhaMahapatra should we merge this? or is this no longer relevant based on our latest discussions?

@AyanSinhaMahapatra

AyanSinhaMahapatra commented Aug 6, 2021

Copy link
Copy Markdown
Member

@pombredanne Yes, this is not relevant anymore. @akugarg Thanks though.

@pombredanne pombredanne closed this Aug 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AyanSinhaMahapatra AyanSinhaMahapatra AyanSinhaMahapatra requested changes
@pombredanne pombredanne pombredanne requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Unknown Licenses should be reported separately

3 participants

@akugarg @AyanSinhaMahapatra @pombredanne