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.

Sign up for GitHub

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

Jump to bottom

Improve License Detection reporting #3286

Merged
merged 12 commits into from
Mar 20, 2023
Merged

Improve License Detection reporting #3286

merged 12 commits into from
Mar 20, 2023

Conversation

AyanSinhaMahapatra
Copy link
Member

@AyanSinhaMahapatra AyanSinhaMahapatra commented Mar 8, 2023 •
edited by pombredanne
Loading

Fixes #3266
Fixes #3269

This PR improves the License Detection output with important and cosmetic changes:

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
Move detection log after matches in LicenseDetection
7e50e17 
Reference: #3266
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Remove 'not-combined' from LicenseDetection logs
d61ae7d 
Reference: #3266
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Modify Misc LicenseDetection attributes
b34198e 
- Rename count to detections_count in UniqueLicenseDetection
- Add rule_relevance back to match level data

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Update test expecatations for LicenseDetection changes
5c57de5 
Updates test expectations for a few LicenseDetection cosmatic changes.

- Remove 'not-combined'
- Move detection log after matches
- Add rule relevance
- rename count to detection count

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Merge branch 'develop' into license-detection-follow-up
69ccacc 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Only show detection logs on diagnostics option
504434b 
Only show license detection logs when --license-text-diagnostics is enabled.

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Update test expectations for detection logs
964cef9 
Update test expectations after making detection logs only present in
--license-text-diagnostics.

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra AyanSinhaMahapatra force-pushed the license-detection-follow-up branch from 43b8ebf to 964cef9 Compare March 8, 2023 16:33
@AyanSinhaMahapatra
Make license references a CLI option #3269
84eda21 
Moved license references to a CLI option and not default.

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Regen test expectations after removing license references as default
a7ee549 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra AyanSinhaMahapatra force-pushed the license-detection-follow-up branch from fdb1992 to a7ee549 Compare March 9, 2023 11:56
@pombredanne pombredanne changed the title Follow up LicenseDetection updates Improve License Detection reporting Mar 9, 2023
pombredanne
pombredanne requested changes Mar 9, 2023
View reviewed changes
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 overall. Thanks!
Let's have a chat at your convenience though as I think we can do better wrt. inlining or not inlining license matches.

tests/licensedcode/data/additional_licenses/additional_license_combined_test.expected.json Outdated Show resolved Hide resolved
tests/licensedcode/data/additional_licenses/additional_license_combined_test.expected.json Show resolved Hide resolved
@AyanSinhaMahapatra
Add --license-diagnostics CLI option
b3a1192 
Adding a --license-diagnostics options for showing detection logs,
which are now disabled by default. This functionality was added to
--license-text-diagnostics previously, but with this commit it is a
separate CLI option.

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Add identifier to resource/package license detections
34b2acd 
* Top level license detections are lean, without matches
* Resource/package level license detections have the identifier
* identifier is computed scan time, and recomputed in post-processing
  if detections are combined.

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Regen test expectations after identifier update
2acfd63 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra AyanSinhaMahapatra force-pushed the license-detection-follow-up branch from ded31de to 2acfd63 Compare March 14, 2023 08:45
@AyanSinhaMahapatra
Copy link
Member Author

@pombredanne This is ready for review!

pombredanne
pombredanne requested changes Mar 20, 2023
View reviewed changes
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.

This is weird... as there should be no detection_log at the top

...ensedcode/data/licenses_reference_reporting/license-reference-works-with-clues.expected.json Show resolved Hide resolved
pombredanne
pombredanne reviewed Mar 20, 2023
View reviewed changes
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 otherwise and ready to merge!

@AyanSinhaMahapatra
Copy link
Member Author

Thanks @pombredanne, merging this!

@AyanSinhaMahapatra AyanSinhaMahapatra merged commit 1f88f60 into develop Mar 20, 2023
@AyanSinhaMahapatra AyanSinhaMahapatra deleted the license-detection-follow-up branch March 20, 2023 14:14
@AyanSinhaMahapatra AyanSinhaMahapatra mentioned this pull request Jul 3, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pombredanne pombredanne pombredanne requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@AyanSinhaMahapatra @pombredanne