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

Content Identifiers for Software Artifacts #701

Merged
merged 5 commits into from
Apr 11, 2024
Merged

Conversation

zvr
Copy link
Member

@zvr zvr commented Apr 8, 2024

This adds a general system for supporting various content identifiers for Software Artifacts.

It adds a new property on SoftwareArtifact, its class, and a vocabulary for different types of identifiers (which can be expanded in the future, with no breaking changes).

This is re-work of #611

Signed-off-by: Alexios Zavras (zvr) <github@zvr.gr>
@zvr zvr requested review from goneall and kestewart April 8, 2024 19:22
Signed-off-by: Alexios Zavras (zvr) <github@zvr.gr>
@rnjudge
Copy link
Collaborator

rnjudge commented Apr 9, 2024

Discussion on the tech call 4/9 favored removing the gitoid property in favor of this generalized type.

Copy link
Member

@maxhbr maxhbr left a comment

Choose a reason for hiding this comment

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

LGTM

model/Software/Classes/ContentIdentifier.md Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove gitoid as it is redundant and confusing to implementers

Copy link
Member Author

Choose a reason for hiding this comment

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

@JPEWdev do you want me to do it in this PR, or do you want to approve this one and I can create a new one?

Copy link
Contributor

Choose a reason for hiding this comment

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

A separate PR is fine, but it is a breaking 3.0 change so it needs to be resolved before release

@rnjudge
Copy link
Collaborator

rnjudge commented Apr 9, 2024

@jeff-schutt will review after the call today then we can merge with changes above.

Co-authored-by: Maximilian Huber <maximilian.huber@tngtech.com>
@jeff-schutt
Copy link
Collaborator

This PR goes against a prior agreement, documented here:

Looks good to me. I did verify that we did decide on gitoid as the content identifier in the Nov 1, 2022 tech call.

#381 (review)

@zvr
Copy link
Member Author

zvr commented Apr 10, 2024

It does not "go against" something discussed in 2022, since it still allows using a gitoid as a content identifier.

Signed-off-by: Alexios Zavras (zvr) <github@zvr.gr>
@JPEWdev
Copy link
Contributor

JPEWdev commented Apr 10, 2024

Nevermind, we did say AnyURI was OK

Copy link
Contributor

@kestewart kestewart left a comment

Choose a reason for hiding this comment

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

This lets a gitoid be considered a content identifier, and lets other standard content identifers that may emerge be incorporated in a structured non breaking way in future.

@rnjudge
Copy link
Collaborator

rnjudge commented Apr 11, 2024

Discussed with Kate offline, this PR is ready to merge.

@rnjudge rnjudge merged commit 3090151 into spdx:main Apr 11, 2024
1 check passed
@zvr zvr deleted the content-identifier branch August 12, 2024 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants