image-label: JSON schemas and clarifications#130
Merged
Conversation
Try to be unambiguous and explicit about the colors, properties and image key
Contributor
Automated Review URLs |
k-dominik
reviewed
Jun 15, 2022
Member
Argh. Again not working. Otherwise, 👍 though I agree that the schema name is not particularly friendly. |
Member
Author
Open to alternative suggestions but my concern is that this might force us to modify the name of existing published schemas - https://github.com/ome/ngff/tree/main/0.4/schemas. |
will-moore
reviewed
Jun 23, 2022
will-moore
reviewed
Jun 23, 2022
will-moore
reviewed
Jun 23, 2022
will-moore
reviewed
Jun 23, 2022
will-moore
reviewed
Jun 23, 2022
Member
|
LGTM 👍 |
Member
Author
|
Re the remaining sticking point (the name), considering simply naming the schemas as |
Member
|
Those names sound fine to me 👍 |
joshmoore
approved these changes
Jul 11, 2022
Member
joshmoore
left a comment
There was a problem hiding this comment.
- Generally 👍
- I imagine there are some downstream uses that might need updates: e.g. https://github.com/ome/ome-ngff-validator/blob/0985fcc2d360c9cf82b51dbefba1320a885021a0/src/JsonKeyLink.svelte#L20
- For colors/labels/etc., I assume we are increasingly approaching a reworking.
Member
Member
Author
|
Yes and furthermore it only fixes #125. The previous description was erroneous (now updated) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #125
This PR reviews the
image-labelspecification and brings it inline with recent improvements made to other specifications including:In addition to reviewing the validation tests keep passing and that the modified text is sensible, the two biggest changes that should be reviewed, notably by the original authors of the specification (all marked as reviewers of this PR) are:
colorsandpropertieskeys with no requirement level and does not define theversionkey. In addiition to clarifying the expected content, this PR proposes a set of requirement levels using the keywords from RFC2119 and implemented through the JSON schemasstrict_image-label.schemato follow the convention used for other JSON schemas while keeping the hyphen separator using inimage-label. The mixture of underscore and hyphen is most likely confusing. Should we change to underscores?Update (2022-07-05): updated the proposal to use
label.schemaandstrict_label.schema