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

Add licenseListVersion to LicenseExpression #481

Merged
merged 4 commits into from
Aug 25, 2023
Merged

Add licenseListVersion to LicenseExpression #481

merged 4 commits into from
Aug 25, 2023

Conversation

goneall
Copy link
Member

@goneall goneall commented Aug 18, 2023

Fixes #131

This adds the licenseListVersion which in 2.3 was at the document level to the licenseExpression.

This is alternative solution to PR #480

Fixes #131

This adds the licenseListVersion which in 2.3 was at the document
level to the licenseExpression.

Signed-off-by: Gary O'Neall <gary@sourceauditor.com>
@goneall goneall marked this pull request as draft August 18, 2023 16:44
Signed-off-by: Gary O'Neall <gary@sourceauditor.com>
@goneall
Copy link
Member Author

goneall commented Aug 19, 2023

After discussing with @swinslow - this seems to be the preferred approach to adding licenseListVersion - changing from draft to "ready for review"

@zvr - please review

@goneall goneall marked this pull request as ready for review August 19, 2023 00:30
model/SimpleLicensing/Properties/licenseListVersion.md Outdated Show resolved Hide resolved
model/SimpleLicensing/Properties/licenseListVersion.md Outdated Show resolved Hide resolved

- 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

Signed-off-by: Gary O'Neall <gary@sourceauditor.com>

- 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'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.

Signed-off-by: Gary O'Neall <gary@sourceauditor.com>
@goneall goneall merged commit 2f978cd into main Aug 25, 2023
1 check passed
@goneall goneall deleted the issue131 branch August 25, 2023 21:20
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.

License Profile: add licenseListVersion to Creation Information
3 participants