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

Update SPDX predicate specification #187

Merged

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Apr 12, 2023

This commit contains a suggestion for updating the SPDX predicate specification, to include the version in the predicateType.

The motivation for this change is same to the motivation that was mentioned for including the version in the CycloneDX predicate specification which is that it allows a consumer to determine the version without having to parse the predicate itself.

@danbev danbev requested a review from a team as a code owner April 12, 2023 12:09
This commit contains a suggestion for updating the SPDX predicate
specification, to include the version in the predicateType.

The motivation for this change is same to the motivation that was
mentioned for including the version in the CycloneDX predicate
specification which is that it allows a consumer to determine the
version without having to parse the predicate itself.

Signed-off-by: Daniel Bevenius <[email protected]>
@danbev danbev force-pushed the spdx-predicatetype-version-suggestion branch from eaef337 to 89d76d6 Compare April 12, 2023 12:11
Copy link
Contributor

@joshuagl joshuagl left a comment

Choose a reason for hiding this comment

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

Thanks for the suggestion, I completely agree with the rationale.

@@ -1,47 +1,69 @@
# Predicate type: SPDX

Type URI: (tentative) https://spdx.dev/Document
Type URI: https://spdx.dev/Document
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we somehow indicate that the Type URI MUST include a version?

Suggested change
Type URI: https://spdx.dev/Document
Type URI: https://spdx.dev/Document/v${SPDX_MAJOR}.${SPDX_MINOR}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense to me 👍

It would probably makes sense to also update the CycloneDX predicate specification in that case, as it does not include the version in the Type URI:

Type URI: https://cyclonedx.org/bom 

Copy link
Contributor

Choose a reason for hiding this comment

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

I like this idea, though we haven't required this in other predicate definitions either.

This may be a question beyond the scope of this PR, but it seems like we might want to be clearer in our documentation for predicateTypes.

spec/predicates/spdx.md Show resolved Hide resolved
Add the removed TODO.

Signed-off-by: Daniel Bevenius <[email protected]>
Copy link
Contributor

@marcelamelara marcelamelara left a comment

Choose a reason for hiding this comment

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

Thanks for the updates @danbev !

Copy link
Contributor

@joshuagl joshuagl left a comment

Choose a reason for hiding this comment

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

LGTM, thank you

@marcelamelara marcelamelara merged commit 4a2df3b into in-toto:main Apr 12, 2023
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.

3 participants