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

feat(shared-data): create v3 JSON protocol schema def #3307

Closed
wants to merge 1 commit into from

Conversation

IanLondon
Copy link
Contributor

@IanLondon IanLondon commented Apr 5, 2019

overview

Closes #3110 - see this ticket for list of changes from v2

changelog

  • add new JSON Schema def for v3 JSON Protocols

review requests

exampleV3Protocols.zip

  • Check out the schema, make sure it's designed in a sensible way that accomplishes the things we've talked about and doesn't block anything we need
  • Play with the example v3 protocol files and make sure the schema validates them when they should and fails to validate "bad" protocol files

npm install -g ajv-cli then you can use the ajv CLI to validate v3 JSON protocols with the new schema:

ajv validate -s shared-data/protocol-json-schema/protocolSchemaV3.json -r shared-data/labware-json-schema/labware-schema.json -d path/to/exampleProtocol.json (the -r pulls in the v2 labware schema so it resolve the reference to opentronsLabwareSchemaV2 in the definitions section)

@codecov
Copy link

codecov bot commented Apr 5, 2019

Codecov Report

Merging #3307 into edge will decrease coverage by 0.09%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             edge    #3307     +/-   ##
=========================================
- Coverage   53.63%   53.53%   -0.1%     
=========================================
  Files         716      716             
  Lines       20907    20959     +52     
=========================================
+ Hits        11213    11221      +8     
- Misses       9694     9738     +44
Impacted Files Coverage Δ
...-library/src/components/LabwareList/LabwareCard.js 25.8% <0%> (-10.78%) ⬇️
components/src/tooltips/HoverTooltip.js 9.09% <0%> (-4.25%) ⬇️
components/src/buttons/Button.js 100% <0%> (ø) ⬆️
...abware-library/src/components/LabwareList/index.js 100% <0%> (ø) ⬆️
...ware-library/src/components/LabwareRender/index.js 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f6789cb...73c73a0. Read the comment docs.

Copy link
Contributor

@b-cooper b-cooper left a comment

Choose a reason for hiding this comment

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

It looks good 🏋️. Hard to see the changes without a diff. I'll diff the files locally.

"slot": {
"description": "Slot on the deck of an OT-2 robot",
"type": "string",
"enum": ["1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, how hard would it be to have this reference the deck schema? This might be a nice change to start moving towards minimizing the sources of truth for slots.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm... the deckSchema.json schema doesn't have a slot enum, that info isn't in the deck schema itself. I don't know if JSON Schema has a way to extract these strings from an ordinary data JSON (eg shared-data/robot-data/decks/ot2Standard.json), I think it can only reference other JSON schemas.

Copy link
Contributor

Choose a reason for hiding this comment

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

Aha that makes sense, good point.

"type": "string"
},
"data": {
"description": "Any data used by the application that created this protocol)",
Copy link
Contributor

Choose a reason for hiding this comment

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

is that an extra closing paren?

@IanLondon
Copy link
Contributor Author

Closed in favor of #3312 which introduces additional changes to v3

@IanLondon IanLondon closed this Apr 5, 2019
@IanLondon IanLondon deleted the shared-data_jsonProtocolV3-def branch May 8, 2019 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Write new JSON schema for V3
2 participants