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(app, protocol-designer, step-generation): schema v8 migration #13884

Merged
merged 10 commits into from
Oct 31, 2023

Conversation

jerader
Copy link
Collaborator

@jerader jerader commented Oct 31, 2023

closes RAUT-828

Overview

This PR migrated the types in PD, step-generation, and the app to schema v8.

Test Plan

Create a protocol in PD without any deck configuration (no waste chute, staging area slots) and download. Upload it to the app. The app should function as expected and the protocol should say creation method PD 8.0.

Screen Shot 2023-10-31 at 2 03 53 PM

Changelog

  • export v8 types in the index file in shared-data and update all instances of v7 to v8 except for when it isn't needed.
  • deprecate usage of ProtocolAnalysisFile
  • update fileCreator in PD to match the new schema shape
  • update 7_1_0 PD migration file to say 8_0_0 instead for a v8 migration.

Note: PD migration cypress test will be updated/thoroughly tested in a follow up PR!

Review requests

see test plan

Risk assessment

low

@jerader jerader requested review from a team as code owners October 31, 2023 18:34
@jerader jerader requested a review from a team October 31, 2023 18:34
@jerader jerader requested a review from a team as a code owner October 31, 2023 18:34
@jerader jerader requested review from smb2268, sfoster1 and b-cooper and removed request for a team October 31, 2023 18:34
Comment on lines -128 to -133
export function protocolHasLiquids(
protocol: ProtocolAnalysisFile<{}> | ProtocolAnalysisOutput
): boolean {
return 'liquids' in protocol && size(protocol.liquids) > 0
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this util isn't being used anymore

Copy link

codecov bot commented Oct 31, 2023

Codecov Report

Merging #13884 (8c1f666) into edge (446d977) will decrease coverage by 0.01%.
Report is 6 commits behind head on edge.
The diff coverage is 92.30%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #13884      +/-   ##
==========================================
- Coverage   70.86%   70.86%   -0.01%     
==========================================
  Files        2501     2501              
  Lines       70386    70388       +2     
  Branches     8613     8612       -1     
==========================================
- Hits        49879    49878       -1     
  Misses      18404    18404              
- Partials     2103     2106       +3     
Flag Coverage Δ
app 68.85% <ø> (-0.03%) ⬇️
g-code-testing 96.44% <ø> (ø)
notify-server 89.13% <ø> (ø)
protocol-designer 45.51% <90.00%> (+0.03%) ⬆️
react-api-client 65.82% <ø> (ø)
shared-data 72.96% <100.00%> (-0.01%) ⬇️
step-generation 84.95% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
app/src/App/hooks.ts 10.34% <ø> (ø)
...c/organisms/CommandText/TemperatureCommandText.tsx 71.42% <ø> (ø)
...nisms/Devices/HeaterShakerIsRunningModal/index.tsx 94.73% <ø> (ø)
...es/ProtocolRun/SetupFlexPipetteCalibrationItem.tsx 87.09% <ø> (ø)
...vices/ProtocolRun/SetupLabware/LabwareListItem.tsx 73.75% <ø> (ø)
...ces/ProtocolRun/utils/getInitialLabwareLocation.ts 0.00% <ø> (ø)
...ms/Devices/ProtocolRun/utils/getLabwareLocation.ts 77.77% <ø> (ø)
.../Devices/ProtocolRun/utils/getLabwareRenderInfo.ts 80.00% <ø> (ø)
...ices/ProtocolRun/utils/getModuleInitialLoadInfo.ts 75.00% <ø> (ø)
...otocolRun/utils/getPickUpTipCommandsWithPipette.ts 100.00% <ø> (ø)
... and 31 more

... and 3 files with indirect coverage changes

Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

Looks good to me and fixes an issue where I couldn't add a new command to command schema 8 without things breaking!

@sfoster1 sfoster1 requested a review from a team as a code owner October 31, 2023 19:49
@sfoster1
Copy link
Member

Removed deck schema specification on request of rss here

Copy link
Contributor

@smb2268 smb2268 left a comment

Choose a reason for hiding this comment

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

No red flags for me! I tested this with a simple protocol and upload to the app worked as expected on a dev app build on this branch.

@jerader jerader merged commit 916e2a7 into edge Oct 31, 2023
58 checks passed
@jerader jerader deleted the app_pd-schema-8-migration branch October 31, 2023 21:46
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.

3 participants