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.

Sign up for GitHub

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

Jump to bottom

Add licenseListVersion to LicenseExpression #481

Merged
merged 4 commits into from
Aug 25, 2023
Merged

Add licenseListVersion to LicenseExpression #481

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
848b8af
Add licenseListVersion to LicenseExpression
goneall Aug 17, 2023
03d6eb7
Add optional cardinality to licenseListVersion
goneall Aug 19, 2023
07c57c6
Update text of LicenseListVersion per comments
goneall Aug 19, 2023
5d6953d
Change licenseListVersionType to /Core/SemVer
goneall Aug 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions model/SimpleLicensing/Classes/LicenseExpression.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ SPDX License Expressions provide a way for one to construct expressions that mor
- type: xsd:string
- minCount: 1
- maxCount: 1
- licenseListVersion
- type: xsd:string
goneall marked this conversation as resolved.
Show resolved Hide resolved
- customIdToUri
- type: /Core/DictionaryEntry
- minCount: 0
18 changes: 18 additions & 0 deletions model/SimpleLicensing/Properties/licenseListVersion.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
SPDX-License-Identifier: Community-Spec-1.0

# licenseListVersion

## Summary

The version of the SPDX License List used when the license expression was created.
goneall marked this conversation as resolved.
Show resolved Hide resolved

## Description

Recognizing that licenses are added to the SPDX License List with each subsequent version, the intent is to provide consumers with the version of the SPDX License List used. This anticipates that in the future, license expression might have used a version of the SPDX License List that is older than the then current one.
goneall marked this conversation as resolved.
Show resolved Hide resolved

## Metadata

- name: licenseListVersion
- Nature: DataProperty
- Range: xsd:string
Copy link
Member

Choose a reason for hiding this comment

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

I know I'm opening a new topic, but should we be using /Core/SemVer like the specVersion? @swinslow , what do you think?

Totally fine with ignoring this for now and resolving it later in another issue, if it will add delay.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's leave it as is for now. Would making it /Core/SemVer less "simple"?

Copy link
Member

Choose a reason for hiding this comment

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

I'd say future issue (as in "far future"). I think we've always used Major.Minor versioning for the license list and I don't know of any current reason to change it to Major.Minor.Patch.

Copy link
Member

Choose a reason for hiding this comment

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

Well, we used to use SPDX x.y for the spec number till now and we moved to SemVer; it might be ok to do the same here.

If not, we have to add a pattern for the valid values of this string, i.e making sure they match x.y -- and I think we have said that even single instances of patterns should be separate custom datatypes. Definitely more complicated.

But as I said, this might be a different issue and this can be merged.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree with @zvr that using SemVer would be simpler than creating a different datatype.

@swinslow - if you're OK with the change, I'll update this PR. If not or if you're not sure, I'll merge this and we can open a separate PR to make progress.

Copy link
Member

Choose a reason for hiding this comment

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

@goneall I'm fine with switching to SemVer if you're okay with it. I'm just conscious that it likely means changes to the License List Publisher so I didn't want to commit to it if you have qualms about it!

For the time being I don't expect we'd do anything other than x.y.0 releases, so maybe it's not that big of a deal in any case. But yes, I'm fine with it if you both are good with it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @swinslow for the quick reply - I committed a change to use /Core/SemVer

@zvr - if you could take one more look and approve, I can merge