-
Notifications
You must be signed in to change notification settings - Fork 37
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
License property improvements #497
License property improvements #497
Conversation
Co-authored-by: Antanas Vaitkus <antanas.vaitkus90@gmail.com>
Co-authored-by: Antanas Vaitkus <antanas.vaitkus90@gmail.com>
Co-authored-by: Antanas Vaitkus <antanas.vaitkus90@gmail.com>
…that the licenses included here are additions to `available_licenses` for data only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a few proposals that I think helps improve the clarity and stay clear of things possible to be interpreted in unintentional ways.
Co-authored-by: Rickard Armiento <gitcommits@armiento.net>
Co-authored-by: Rickard Armiento <gitcommits@armiento.net>
Co-authored-by: Rickard Armiento <gitcommits@armiento.net>
Co-authored-by: Rickard Armiento <gitcommits@armiento.net>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I was holding off for other conversions to settle; but I think the current version is good enough to merge. Thanks for the work on this!
Thanks @rartino. It would be nice to get approvals from more OPTIMADE adopters as this PR changes the interpretation of license properties (new in this release, though). |
@ml-evs In the spirit of cutting down on our PR log - since you've commented on this previously - do you still think there are remaining issues, or could you consider approving it?
This PR clarifies a few situations that were previously unclear, and extends what can be expressed. But as far as I see, no behavior that was well-defined with the previous PR changes by this PR. (Feel free to enlighten me if I'm missing something.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the current wording is clear to me (and circumvents my need for examples). Thanks all!
@merkys Are you comfortable 'resolving' the two remaining conversations and merging this? (Apparently leaving conversations unresolved is now a blocker that cannot be overridden - I'm not sure how I feel about that.) |
This is correct. I guess I tried to be extra-safe here with talking about the changes to the interpretation. I have just marked all the remaining conversations about the text as resolved. |
This PR addresses issues raised in #494 by:
available_licenses
means;available_licenses
;available_licenses_for_entries
to specify licenses for database content and metadata only (without its structure).Fixes #494.