Skip to content
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

More License Detection changes #3154

Merged
merged 11 commits into from
Dec 24, 2022
Merged

Conversation

AyanSinhaMahapatra
Copy link
Member

@AyanSinhaMahapatra AyanSinhaMahapatra commented Nov 21, 2022

  • Creates a top-level licenses attribute with unique license detections
  • makes the licenses-reference default, and instead adds a --no-licenses-reference option

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 📁

- Add a new codebase level attribute `licenses`
- Add a new resource level attribute `for_licenses`
- Add unique license detections from files and packages
  in the top level attribute `licenses` and this is the
  usual `license_expression`, `detection_log` and `matches`
  and additionally an `occurance_count` and a `identifier`
  which is an UUID generated from the content of the matches
  in the detection.

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

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

Thanks!
See some comments inline for your review.

* removes the `--licenses-reference` CLI option and plugin
* the license_references and license_rule_references attributes
  are now default with `--license` option and the same has been removed
  from match level data to avoid data duplication.
* tests are reorganized and files renamed

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
* Due to license references being default, reference data isn't
  inlined anymore, so we need to use the cache to get this data
  and also rehydrate them into objects to be able to post process
  license related info.
* Use license objects wherever possible instead of mappings.

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
* rename top level attirbute `licenses` -> `license_detections`
* rename top level attribute `rule_references` to `license_rule_references`
* include license_expression in identifier
* use the correct spelling for occurrance
* include matched_text in matches instead of reference data
* file level attribute `for_licenses` changed to `for_license_detections`
* fix bugs including license rule references correctly
* make license references default to `--licenses`

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
* Reorder codebase and resource level attributes
* replace `#` in license detection identifier with `-`
* regenerate test expectations
* reorder license rule references attributes
* add rule text to license rule references data

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
* Add rule text to reference data
* Add rule url to reference data
* make rule references unique
* reorder rule references data
* regenerate test expectations

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra AyanSinhaMahapatra changed the title More License Detection changes [WIP] More License Detection changes Dec 20, 2022
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Copy link
Member Author

#3150 now has all the changes from this PR.

Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

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

LGTM! merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants