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 licenseXml property to License and LicenseAddition #451

Merged
merged 1 commit into from
Aug 13, 2023
Merged

Conversation

goneall
Copy link
Member

@goneall goneall commented Jul 30, 2023

Resolves #359

@goneall goneall requested review from zvr and swinslow July 30, 2023 01:33
@kestewart kestewart added this to the 3.0-rc2 milestone Jul 31, 2023
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.

Looks good to me.

Resolves #359

Signed-off-by: Gary O'Neall <gary@sourceauditor.com>
@goneall
Copy link
Member Author

goneall commented Aug 12, 2023

@swinslow - If you could do a quick review, I can merge if it looks OK

@swinslow
Copy link
Member

@goneall My only question is whether the property on LicenseAddition should be called additionXml rather than licenseXml? To help clarify that it is not identical to a "license" XML file.

I'm not going to lose sleep over this one if you feel strongly that they should be the same :)

@goneall
Copy link
Member Author

goneall commented Aug 12, 2023

My only question is whether the property on LicenseAddition should be called additionXml rather than licenseXml? To help clarify that it is not identical to a "license" XML file.

@swinslow Your suggestion makes sense to me, however as I was starting to make the changes and reading the text of the licenseXml.md file, I notice it points to the license XML fields which documents both the "license" and "exception" fields.

Since it is pointing to the same XML format for both license and addition, I'm wondering if they should keep the same term. It does seem a bit odd to have something called licenseXml in the addition. Let me know what you think. Should we leave it as is, change the name of the field to something more appropriate or change the documentation for license fields to define two different format?

@swinslow
Copy link
Member

Yeah.... we refer to it generally as the "license list XML" -- I think we can keep it as licenseXml unless / until someone complains. I think it's fine. Thanks @goneall!

@goneall goneall merged commit 27371c9 into main Aug 13, 2023
1 check passed
@goneall goneall deleted the issue359 branch August 13, 2023 17:10
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.

Add a licenseXML field to the License class
3 participants