Issue 503 allow list with noassertion #505
Conversation
meretp
force-pushed
the
issue-503-allow-list-with-noassertion
branch
3 times, most recently
from
47b7081 to
b0c73a0
Compare
armintaenzertng
left a comment
armintaenzertng
left a comment
There was a problem hiding this comment.
Thanks for this!
I found a few places that still can be adapted. Also, we can simplify the file_converter, package_converter and snippet_converter files in the jsonschema package.
|
|
||
| def validate_license_expressions(license_expressions: Optional[ | ||
| Union[List[LicenseExpression], SpdxNoAssertion, SpdxNone]], document: Document, parent_id: str) -> List[ValidationMessage]: | ||
| if license_expressions in [SpdxNoAssertion(), SpdxNone(), None]: |
There was a problem hiding this comment.
The None case is missing now in the new code
There was a problem hiding this comment.
There is no None case, only an empty list, I fixed the type hint.
| Union[List[LicenseExpression], SpdxNoAssertion, SpdxNone]] = parse_field_or_log_error( | ||
| List[Union[LicenseExpression, SpdxNoAssertion, SpdxNone]]] = parse_field_or_log_error( | ||
| logger, file_dict.get("licenseInfoInFiles"), | ||
| lambda x: parse_field_or_no_assertion_or_none(x, self.license_expression_parser.parse_license_expressions)) |
There was a problem hiding this comment.
We still use parse_field_or_no_assertion_or_none here; I don't think that is needed anymore, as parse_license_expressions only returns lists now. Use field_is_list=True instead?
See also the package and snippet parser.
| checksums = [checksum_fixture()] if checksums is None else checksums | ||
| license_info_from_files = [get_spdx_licensing().parse("MIT"), get_spdx_licensing().parse( | ||
| "GPL-2.0")] if license_info_from_files is None else license_info_from_files | ||
| "GPL-2.0"), SpdxNoAssertion()] if license_info_from_files is None else license_info_from_files |
There was a problem hiding this comment.
I'd like to have one of these SpdxNoAssertion()s be SpdxNone(), just to have that covered in the tests, too.
| <spdx:Package rdf:about="https://some.namespace#SPDXRef-Package2"> | ||
| <spdx:name>packageName</spdx:name> | ||
| <spdx:releaseDate>2022-12-01T00:00:00Z</spdx:releaseDate> | ||
| <spdx:supplier>Person: supplierName ([email protected])</spdx:supplier> |
There was a problem hiding this comment.
What's the reason to delete almost everything from this package in the first place but then not deleting it fully?
There was a problem hiding this comment.
We use the second package to test different values in ExternalPackageRef (see test_package_parser.py). While working on this I decided that we don't need all these other fields in the second package and deleted it. I admit that this is not really related to the issue...
armintaenzertng
left a comment
armintaenzertng
left a comment
There was a problem hiding this comment.
Thanks, only a small remark.
| license_concluded = parse_field_or_log_error( | ||
| logger, package_dict.get("licenseConcluded"), | ||
| lambda x: parse_field_or_no_assertion_or_none(x, self.license_expression_parser.parse_license_expression), | ||
| None) | ||
| logger, package_dict.get("licenseConcluded"), self.license_expression_parser.parse_license_expression, None) |
There was a problem hiding this comment.
The None value is default anyway and license_concluded has not type hint
meretp
force-pushed
the
issue-503-allow-list-with-noassertion
branch
from
d107739 to
b5df9e1
Compare
…ression, SpdxNone or SpdxNoAssertion Signed-off-by: Meret Behrens <[email protected]>
Signed-off-by: Meret Behrens <[email protected]>
Signed-off-by: Meret Behrens <[email protected]>
Signed-off-by: Meret Behrens <[email protected]>
… cases for LicenseInfoFromFiles, LicenseInfoInFile and LicenseInfoInSnippet Signed-off-by: Meret Behrens <[email protected]>
meretp
force-pushed
the
issue-503-allow-list-with-noassertion
branch
from
b5df9e1 to
e8aa932
Compare
2 participants
fixes #503