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 support for $comment in property definitions #459

Closed
rartino opened this issue Feb 23, 2023 · 4 comments
Closed

Add support for $comment in property definitions #459

rartino opened this issue Feb 23, 2023 · 4 comments

Comments

@rartino
Copy link
Contributor

rartino commented Feb 23, 2023

JSON schema has in draft 7 added support to put comments in schemas via $comment: http://json-schema.org/understanding-json-schema/reference/generic.html#comments

When working on our property definitions in #445 I independently realized it would be useful to insert comments in the property definitions. So we should add this to the subset of JSON Schema that we allow.

@merkys
Copy link
Member

merkys commented Feb 23, 2023

What would be the target audience of comments? Maybe we can just put them to description?

@rartino
Copy link
Contributor Author

rartino commented Feb 23, 2023

It would be for comments that are relevant to property definition "developers" rather than "end users". Another difference is that you can put it on any level in a nested definition to comment e.g. the motivation behind setting a limit like "maximum" to something. I stumbled on the desire to insert a comment when I realized I wanted somewhere to explain why common/element.yaml doesn't have an x-optimade-property (it is because it isn't itself part of the OPTIMADE standard properties, it is just used as a target for '$ref' when we compose other standard properties.)

On the other hand - writing this up makes me realize that we might want to specifically have a way to strip certain comments when we inline '$ref' references, since the comment I mention above would be nonsensical after inlining the definition.

@merkys
Copy link
Member

merkys commented Feb 27, 2023

OK, this makes sense.

@ml-evs
Copy link
Member

ml-evs commented Mar 25, 2024

Closed by #457 (I think) or a related PR

@ml-evs ml-evs closed this as completed Mar 25, 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

No branches or pull requests

3 participants