-
Notifications
You must be signed in to change notification settings - Fork 178
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(protocol-designer, step-generation, shared-data): migrate to using pipette v2 schema #14713
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## edge #14713 +/- ##
==========================================
+ Coverage 67.33% 67.35% +0.02%
==========================================
Files 2485 2485
Lines 71386 71335 -51
Branches 9030 8999 -31
==========================================
- Hits 48069 48051 -18
+ Misses 21173 21152 -21
+ Partials 2144 2132 -12
Flags with carried forward coverage won't be shown. Click here to find out more.
|
The absence of min & max values just seems to be an oversight and we should add them into the v2 definitions in a follow-up. Let me know if you need any help adding them. |
screen.getByText('Choose tips for Flex 1-Channel 50 μL') | ||
screen.getByText('mock EquipmentOption') | ||
screen.getByText('Choose tips for Flex 1-Channel 1000 μL') | ||
screen.getAllByText('mock EquipmentOption') |
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.
need to use AllByText
instead of getByText
?
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.
yeah with the change in pipette, more equipment options (tiprack options) rendered! had to change it a bit since filtered tips aren't included in the pipette's default tips array right now.
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.
The changes look good to me.
This reverts commit 738bf77.
closes AUTH-52, AUTH-231
Overview
Migrate from using
getPipetteNameSpecs
togetPipetteV2Specs
in PD. This change is necessary both for pipette collision warnings and so flow rates are accurate/reflect the apiThere are a few followup action items:
min
s andmax
s for the flowrates in the v2 schema definitions - this information is needed for PD to accurately have a range defining custom flow rates.Test Plan
Smoke test PD to make sure it works as expected. In particular, test these areas:
Changelog
getPipetteNameSpecs
togetPipetteV2Specs
Review requests
see test plan
Risk assessment
low