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

Clarify types and requirements levels for the plate/well specification #126

Merged
merged 5 commits into from
May 30, 2022

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented May 26, 2022

This commit rewrites the section and ensures that:

  • each key has a requirement level specified as MUST/SHOULD/MAY
  • the type for the value of each key is explicitly defined

The requirements & types should match the JSON schemas introduced in #120

Staged at http://api.csswg.org/bikeshed/?url=https://raw.githubusercontent.com/sbesson/ngff/plate_specification_2/0.4/index.bs#plate-md

This commit rewrites the section and ensures that:
- each key has a requirement level specified as MUST/SHOULD/MAY
- the type for the value of each key is explicitly defined
Copy link
Member

@melissalinkert melissalinkert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super pedantic comments aside (some of which are things not introduced by this PR), this does read better than before.

0.4/index.bs Show resolved Hide resolved
0.4/index.bs Show resolved Hide resolved
0.4/index.bs Outdated Show resolved Hide resolved
0.4/index.bs Outdated Show resolved Hide resolved
0.4/index.bs Outdated Show resolved Hide resolved
0.4/index.bs Outdated Show resolved Hide resolved
0.4/index.bs Outdated Show resolved Hide resolved
0.4/index.bs Outdated Show resolved Hide resolved
0.4/index.bs Outdated Show resolved Hide resolved
0.4/index.bs Outdated Show resolved Hide resolved
@sbesson
Copy link
Member Author

sbesson commented May 27, 2022

Thanks for the thorough review @melissalinkert. Pedantic comments are very useful. The aim of this PR is to reduce the ambiguity in the definition of these specifications. Pushed a few commits addressing your suggestions and adding the acquisition description to the JSON schema.

I also ported the same text to the latest specification to save myself another PR once this is merged. If additional changes are requested, I'll apply them to both specifications.

Copy link
Member

@melissalinkert melissalinkert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of typos left, 👍 once those are fixed.

0.4/index.bs Outdated Show resolved Hide resolved
latest/index.bs Outdated Show resolved Hide resolved
latest/index.bs Outdated Show resolved Hide resolved
@sbesson sbesson merged commit e26e206 into ome:main May 30, 2022
github-actions bot added a commit that referenced this pull request May 30, 2022
…ion_2

Clarify types and requirements levels for the plate/well specification

SHA: e26e206
Reason: push, by @sbesson

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request May 30, 2022
…ion_2

Clarify types and requirements levels for the plate/well specification

SHA: e26e206
Reason: push, by @joshmoore

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@sbesson sbesson deleted the plate_specification_2 branch June 6, 2022 20:23
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.

2 participants