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

[issue-386] Finish Package validation #461

Merged

Conversation

armintaenzertng
Copy link
Collaborator

fixes #386

Copy link
Collaborator

@meretp meretp left a comment

Choose a reason for hiding this comment

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

Thanks! Two small remarks from my side, not really blocking, so I would approve this as is.

Comment on lines +29 to +31
(PackageVerificationCode("71c4025dd9897b364f3ebbb42c484ff43d00791c", ["/invalid/excluded/file"]),
'file name must not be an absolute path starting with "/", but is: /invalid/excluded/file')
])
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could add one more testcase with an uppercase checksum as you added this in the vlaidation message.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah yes, and I wanted to give checksums the same treatment

validation_messages.append(
ValidationMessage(
f"package must contain no elements if files_analyzed is False, but found {contained_in_package_relationships}",
f"package must contain no elements if files_analyzed is False, but found {combined_relationships}",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Reading this, I was wondering if a package must contain no elements in general or only files? The spec is mostly referring to files and I am not sure, if a package could contain anything other than a file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think it can contain anything else than files. This is not explicitly stated by the spec for relationships, so who knows... I would leave it like that for now.

@armintaenzertng armintaenzertng merged commit a47533c into spdx:refactor-python-tools Feb 1, 2023
@armintaenzertng armintaenzertng deleted the packageValidation branch February 1, 2023 15:32
@armintaenzertng armintaenzertng mentioned this pull request Feb 2, 2023
3 tasks
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