-
Notifications
You must be signed in to change notification settings - Fork 31
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
Added repro flushing event with type and enums #337
Conversation
Will also need to add the embryo flushing event to ExampleURLScheme and Reproduction URL scheme OAS files. |
Removed sire fields, renamed flushingType to flushingMethod
changed icarReproEmbryoFlushingMethod to icarReproEmbryoFlushingMethodType
Not required at this stage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the tags as discussed.
|
||
"required": ["flushingType"], | ||
"properties": { | ||
"flushingType": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you consider renaming this field flushingMethod, and its corresponding enumeration to icarReproEmbryoFlushingMethodType.json? "Method" is more precise than "type" in this instance.
"type": "array", | ||
"description": "Details of multiple sires used to fertilize the embryo", | ||
"items": { | ||
"$ref": "../types/icarReproPotentialSireIdentifierType.json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good. We should have defined a sire identifier type from the outset - it could have been incorporated in all the places where we have SireIdentifiers+SireOfficialName+SireURI. Given the field is called potentialSires, could the type just be called icarReproSireIdentifierType.json and then in time we can standardise its usage elsewhere (even in the mating recommendation, etc)?
url-schemes/exampleUrlScheme.json
Outdated
"summary": "Get the embryo flushing events for a certain location", | ||
"description": "# Purpose\nProvides the embryo flushing events on a location\n", | ||
"tags": [ | ||
"ADE-1.3-reproduction" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"ADE-1.3-reproduction" | |
"ADE-1.4-reproduction" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requested change has been made to scheme
"summary": "Get the embryo flushings for a certain location", | ||
"description": "# Purpose\nProvides the animal embryo flushings for a location\n", | ||
"tags": [ | ||
"ADE-1.3-reproduction" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"ADE-1.3-reproduction" | |
"ADE-1.4-reproduction" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change has been made to scheme
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
Realised after pressing confirm merge that the target for this PR was the ADE1 branch, not the Develop branch. Have reverted and will need to reapply this PR to the Develop branch (once I figure out how). |
Submitting a flushing event with supporting type and enum for issue #296