Misc. SPDX / license mapping related improvements #8730
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8730 +/- ##
=========================================
Coverage 67.79% 67.79%
Complexity 1164 1164
=========================================
Files 243 243
Lines 7711 7711
Branches 861 861
=========================================
Hits 5228 5228
Misses 2127 2127
Partials 356 356
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
sschuberth
force-pushed
the
warn-invalid-license-mapping
branch
from
19cfe2a to
4bcff23
Compare
|
@oss-review-toolkit/kotlin-devs can we agree that it makes sense to merge this now independently of the outcome of the discussion in #8691? |
sschuberth
force-pushed
the
warn-invalid-license-mapping
branch
from
4bcff23 to
054d4c3
Compare
I've dropped the actual warning log from the PR, but the other small refactoring commits can still be merged IMO. Please have a look @mnonnenmacher and / or @fviernau. |
sschuberth
changed the title
sschuberth
force-pushed
the
warn-invalid-license-mapping
branch
2 times, most recently
from
3498e91 to
cf666d9
Compare
| * Parses the string as an [SpdxExpression] of the given [strictness] and returns the result on success, or null if the | ||
| * string cannot be parsed. | ||
| */ | ||
| fun String.toSpdxOrNull(strictness: Strictness = Strictness.ALLOW_ANY): SpdxExpression? = |
There was a problem hiding this comment.
Parses -> Parse
returns -> return
maybe: the string -> this string
| runCatching { | ||
| toSpdx(strictness) | ||
| }.onFailure { | ||
| logger.debug { "Could not parse license '$this' to SPDX: ${it.collectMessages()}" } |
There was a problem hiding this comment.
As $this is not necessary a license, if it could not be parse, should we just drop the mention of "license" from the log message?
There was a problem hiding this comment.
I've reworded the message.
| * afterwards. | ||
| * Normalize all license IDs using [SpdxSimpleLicenseMapping]. If [mapDeprecated] is `true`, also deprecated IDs are | ||
| * mapped to their current counterparts. The result of this function is not guaranteed to contain only valid IDs. | ||
| * Use [validate] to check the returned [SpdxExpression] for validity afterwards. |
There was a problem hiding this comment.
out of scope: Should this be isValid() instead of validate(), because the latter does not return any value?
There was a problem hiding this comment.
I believe the mention of validate() better matches the normalize() context, but I've now simply mentioned isValid() as well.
|
|
||
| # A mapping from license strings collected from the declared licenses of Open Source packages to SPDX expressions. This | ||
| # mapping only contains license strings which can *not* be parsed by [SpdxExpression.parse], for example because the | ||
| # license names contain white spaces. See [SpdxSimpleLicenseMapping] for a mapping of varied license names. |
There was a problem hiding this comment.
Should it be various instead of varied?
There was a problem hiding this comment.
No, "varied" is correct (and also used in the original text that I copied this from) as it's supposed to mean "variants of license names" and not "several license names".
sschuberth
force-pushed
the
warn-invalid-license-mapping
branch
from
cf666d9 to
4ee94f4
Compare
…censes Introduce an intermediate variable to separate these two steps for clarity and to ease debugging. Signed-off-by: Sebastian Schuberth <[email protected]>
Signed-off-by: Sebastian Schuberth <[email protected]>
…tion Signed-off-by: Sebastian Schuberth <[email protected]>
Signed-off-by: Sebastian Schuberth <[email protected]>
While at it, also mention `isValid` in addition to `validate`. Signed-off-by: Sebastian Schuberth <[email protected]>
The remark about ambiguity due to a missing version became somewhat obsolete with e7247e0. Signed-off-by: Sebastian Schuberth <[email protected]>
…ility Signed-off-by: Sebastian Schuberth <[email protected]>
sschuberth
force-pushed
the
warn-invalid-license-mapping
branch
from
4ee94f4 to
7cdf3ef
Compare
No description provided.