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

License property improvements #497

Merged

Conversation

merkys
Copy link
Member

@merkys merkys commented Jan 17, 2024

This PR addresses issues raised in #494 by:

  • defining what an unknown value for available_licenses means;
  • including the structure of the database among the things that are covered by available_licenses;
  • introducing available_licenses_for_entries to specify licenses for database content and metadata only (without its structure).

Fixes #494.

@merkys merkys requested review from rartino and ml-evs January 17, 2024 09:03
optimade.rst Outdated Show resolved Hide resolved
optimade.rst Outdated Show resolved Hide resolved
optimade.rst Outdated Show resolved Hide resolved
optimade.rst Outdated Show resolved Hide resolved
merkys and others added 4 commits January 17, 2024 11:47
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>
optimade.rst Outdated Show resolved Hide resolved
optimade.rst Outdated Show resolved Hide resolved
@ml-evs ml-evs added this to the v1.2 milestone Jan 17, 2024
Copy link
Contributor

@rartino rartino left a 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.

optimade.rst Outdated Show resolved Hide resolved
optimade.rst Outdated Show resolved Hide resolved
optimade.rst Outdated Show resolved Hide resolved
optimade.rst Outdated Show resolved Hide resolved
merkys and others added 5 commits January 18, 2024 16:35
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>
Copy link
Contributor

@rartino rartino left a 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!

@merkys
Copy link
Member Author

merkys commented Feb 9, 2024

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

@rartino
Copy link
Contributor

rartino commented Feb 14, 2024

@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?

@merkys

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

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

Copy link
Member

@ml-evs ml-evs left a 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!

@rartino
Copy link
Contributor

rartino commented Feb 16, 2024

@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.)

@merkys
Copy link
Member Author

merkys commented Feb 19, 2024

@rartino

@merkys

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

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

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.

@rartino rartino merged commit 6165717 into Materials-Consortia:develop Feb 20, 2024
5 checks passed
@rartino rartino mentioned this pull request Mar 22, 2024
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.

Clarify default licensing value
4 participants