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

Confusion around ExtendableLicense and ExpandedLicensing profile #394

Closed
armintaenzertng opened this issue Jun 22, 2023 · 6 comments
Closed
Milestone

Comments

@armintaenzertng
Copy link
Contributor

I don't understand the inheritance structure in the current Licensing/ExpandedLicense profiles, especially the following part:
Licensing:AnyLicenseInfo --> ExpandedLicense:ExtendableLicense --> Licensing:License
If every License object (that includes CustomLicense and ListedLicense) is a subclass of ExtendableLicense, why are they not part of the ExpandedLicense profile? Same question goes for OrLaterOperator, which is also a subclass of ExtendableLicense.
Alternatively, ExtendableLicense might just be moved to the Licensing profile as it is abstract anyway and does not introduce any new properties as far as I can tell.

If the current state is indeed intended the way it is, I propose to write a bit more documentation on why we cycle through different profile for the inheritance chain above.

@goneall
Copy link
Member

goneall commented Jun 24, 2023

If every License object (that includes CustomLicenseand ListedLicense) is a subclass of ExtendableLicense, why are they not part of the ExpandedLicense profile?

For the "simple" license profile Licensing, we want to allow for a LicenseExpression which requires parsing, a single ListedLicense or a single CustomLicense - hence it should be in the Licensing profile. This is the result of the meeting we had on the complex licensing topic a few weeks back.

The idea is that ExpandedLicense would only be used for more complex license expressions and not requiring parsing of the text LicensingExpression.

As far as the WithOperator and OrLaterOperator, I believe we also wanted those to be part of the simple licensing profile based on the meeting but it does seem to lead to some mixing of the parsed and and string expressions.

Alternatively, ExtendableLicense might just be moved to the Licensing profile as it is abstract anyway and does not introduce any new properties as far as I can tell.

It seems like we should either move ExtendableLicense to the Licensing profile or move the OrLaterOperator and WithOperator to the ExpandedLicense profile.

@zvr @swinslow - thoughts?

@armintaenzertng
Copy link
Contributor Author

or move the OrLaterOperator and WithOperator to the ExpandedLicense profile.

In that case, I think we should also move License, CustomLicense and ListedLicense to the ExpandedLicense profile, as they are all subclasses of ExtendableLicense.

@goneall
Copy link
Member

goneall commented Jun 27, 2023

In that case, I think we should also move License, CustomLicense and ListedLicense to the ExpandedLicense profile, as they are all subclasses of ExtendableLicense.

In that case we should just move ExtendableLicense to the Licensing profile. I think we would want the field definitions of the license to be in the Licensing profile.

@armintaenzertng - would that resolve this issue?

@armintaenzertng
Copy link
Contributor Author

would that resolve this issue?

On a technical level I would say yes.

But I'd be still confused why there is an extra profile that houses ConjunctiveLicenseSet and DisjunctiveLicenseSet, while WithAdditionOperator and OrLaterOperator are in a different profile. I mean, they are all made for the purpose of modeling some keyword from a license expression. Why are "AND" and "OR" in a different profile than "WITH" and "+"?

@goneall
Copy link
Member

goneall commented Jun 27, 2023

Let's discuss on today's tech call if we have time.

@goneall goneall added this to the 3.0-rc2 milestone Jul 12, 2023
@goneall
Copy link
Member

goneall commented Jul 29, 2023

With PR #399 I believe this issue is resolved.

@armintaenzertng if you disagree, please open a new issue

@goneall goneall closed this as completed Jul 29, 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

No branches or pull requests

2 participants