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

refactor(api): Add GEB tiprack definition to default containers #1658

Merged
merged 1 commit into from
Jun 11, 2018

Conversation

Laura-Danielle
Copy link
Contributor

overview

GEB tiprack needs to be added to default containers for customers who are not utilizing
split_definitions feature flag. Also, there was a typo in the docs about the GEB tiprack definition
name.

changelog

  • Add GEB tiprack definition into 'default-containers'
  • Edit docs to reflect correct definition name

review requests

@umbhau
Copy link

umbhau commented Jun 8, 2018

@Laura-Danielle should we call this the opentrons-tiprack-300ul?

Assuming this is for the custom tipracks GEB manufactures for Opentrons, not the off-the-shelf GEB tipracks.

@Laura-Danielle Laura-Danielle force-pushed the api_update-GEB-300-ul-tiprack-info branch from b2c0aa9 to 2e74d3f Compare June 8, 2018 16:15
@Laura-Danielle
Copy link
Contributor Author

@umbhau I am not opposed to changing the name. @btmorr @pantslakz @howisthisnamenottakenyet thoughts?

@codecov
Copy link

codecov bot commented Jun 8, 2018

Codecov Report

Merging #1658 into edge will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge    #1658      +/-   ##
==========================================
+ Coverage   34.62%   34.64%   +0.01%     
==========================================
  Files         340      343       +3     
  Lines        5533     5813     +280     
==========================================
+ Hits         1916     2014      +98     
- Misses       3617     3799     +182
Impacted Files Coverage Δ
protocol-designer/src/file-data/actions.js 50% <0%> (-50%) ⬇️
...-designer/src/top-selectors/well-contents/index.js 26.31% <0%> (-13.69%) ⬇️
protocol-designer/src/file-data/reducers/index.js 29.41% <0%> (-12.26%) ⬇️
...esigner/src/components/IngredientPropertiesForm.js 0% <0%> (ø) ⬆️
...rotocol-designer/src/components/IngredientsList.js 0% <0%> (ø) ⬆️
app-shell/lib/menu.js 0% <0%> (ø) ⬆️
...rotocol-designer/src/containers/IngredientsList.js 0% <0%> (ø) ⬆️
...esigner/src/containers/IngredientPropertiesForm.js 0% <0%> (ø) ⬆️
app/src/components/ToolTip.js 0% <0%> (ø)
protocol-designer/src/file-data/index.js 0% <0%> (ø)
... and 7 more

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 b7cd58c...84831dc. Read the comment docs.

@howisthisnamenottakenyet
Copy link
Contributor

Not opposed to more specific naming.

@btmorr
Copy link
Contributor

btmorr commented Jun 8, 2018

opentrons-tiprack-300ul is fine. Either one works for me, let's just change the documentation to match whatever we decide on

@Laura-Danielle Laura-Danielle force-pushed the api_update-GEB-300-ul-tiprack-info branch from 2e74d3f to f98fa05 Compare June 8, 2018 17:35
@andySigler
Copy link
Contributor

andySigler commented Jun 8, 2018

The more specific naming, in my opinion, the better. If the tip-rack has a part number, that should be either in the name, or specified some other way. It is 100% conceivable that we will have a rev2 or even totally different design of the opentrons-tiprack-300ul

Copy link
Contributor

@btmorr btmorr left a comment

Choose a reason for hiding this comment

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

🍘

@btmorr
Copy link
Contributor

btmorr commented Jun 11, 2018

Pending CI fix

@mcous
Copy link
Contributor

mcous commented Jun 11, 2018

@andySigler raises a really good point about labware revisions. I'm ok with this PR going in (especially to fix up OT2 docs weirdness) but we should discuss how new versions of a given definition are going to play out

GEB tiprack needs to be added to default containers for customers who are not utilizing
split_definitions feature flag. Also, there was a typo in the docs about the GEB tiprack definition
name.
@Laura-Danielle Laura-Danielle force-pushed the api_update-GEB-300-ul-tiprack-info branch from f98fa05 to 84831dc Compare June 11, 2018 16:05
@Laura-Danielle
Copy link
Contributor Author

@mcous @andySigler I think that's definitely something to keep in mind moving forward. We should definitely have a discussion about how to handle revisions/model numbers and where they live.

@Laura-Danielle Laura-Danielle merged commit 0b70873 into edge Jun 11, 2018
@Laura-Danielle Laura-Danielle deleted the api_update-GEB-300-ul-tiprack-info branch June 11, 2018 17:42
@pantslakz
Copy link

happy we're ridding of GEB in place of Opentrons

@btmorr btmorr added the small label Jun 12, 2018
@btmorr btmorr added this to To do in Sprint Week of 2018-06-05 via automation Jun 12, 2018
@btmorr btmorr moved this from To do to Done in Sprint Week of 2018-06-05 Jun 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Affects the `api` project refactor small
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

7 participants