-
Notifications
You must be signed in to change notification settings - Fork 314
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
Scan function refactoring #7788
Conversation
Generalize the wording of the log messages so they can be consolidated in the `scan()` function. Signed-off-by: Sebastian Schuberth <[email protected]>
The `scan()` function also checks the same centrally, so rely on that. Signed-off-by: Sebastian Schuberth <[email protected]>
Resolves #4161. Signed-off-by: Sebastian Schuberth <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7788 +/- ##
=========================================
Coverage 67.84% 67.84%
Complexity 2045 2045
=========================================
Files 357 357
Lines 16770 16770
Branches 2378 2378
=========================================
Hits 11378 11378
Misses 4402 4402
Partials 990 990
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
scannerConfig.detectedLicenseMapping | ||
) | ||
val packages = ortResult.getPackages(skipExcluded).map { it.metadata }.filterNotConcluded() | ||
.filterNotMetadataOnly().toSet() |
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.
Maybe add an empty line here.
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.
I actually had it there first, but then it looked a bit odd compared to the equivalent block with lines 113 / 114, so I decided to not add an empty line here to keep more compact blocks of code. But I don't care that much, I could add an empty line here (and also after line 113) if you prefer.
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.
let's leave it as-is.
val packageResults = scan( | ||
packages, | ||
ScanContext( | ||
ortResult.labels, |
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.
out of scope: Is it correct that line 128 is different from line 116?
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.
Good question. I suspect this should also say ortResult.labels + labels
, @mnonnenmacher?
Please have a look at the individual commit messages for the details.