Y26-680 Automated buffer addition for cherrypicking#5565
Y26-680 Automated buffer addition for cherrypicking#5565andrewsparkes wants to merge 52 commits intodevelopfrom
Conversation
…empty_wells option on Cherrypick batches
…utomatic_buffer_addition checkbox
…es to match expected format
yoldas
left a comment
There was a problem hiding this comment.
Looks good. My comments are notes. After UAT, we might consider describing the options and limitations at https://ssg-confluence.internal.sanger.ac.uk/spaces/PSDPUB/pages/232620342/Cherrypicking+options+in+Sequencescape .
|
There's a lot of code :) any chance of a brief summary / description of changes, and a screenshot or two of the UI changes? Thanks Edit by Andrew as can't seem to reply: see Abdullah's original story with the screenshots here |
KatyTaylor
left a comment
There was a problem hiding this comment.
Didn't manage to finish reviewing, will look again Monday
KatyTaylor
left a comment
There was a problem hiding this comment.
All just suggestions, although don't understand the 1, on bit. Also I haven't reviewed the tests I'm afraid.
app/models/cherrypick/task/buffer_volume_for_empty_wells_option.rb
Outdated
Show resolved
Hide resolved
app/models/cherrypick/task/buffer_volume_for_empty_wells_option.rb
Outdated
Show resolved
Hide resolved
| buffer_entry = buffer_mapping_for_empty_well(opts[:plate], opts[:index], opts[:plate_size], | ||
| opts[:buffer_volume_for_empty_wells]) | ||
| mapping << buffer_entry if buffer_entry | ||
| end |
There was a problem hiding this comment.
Thank you for refactoring (and making the other changes).
I think it's generally more readable. Think this particular method mapping_or_buffer_entry is a little abstract and maybe I would just put this logic into the build_buffer_mapping method.
Happy to approve anyway though.
…id filling wells that are supposed to be empty according to the template
Closes #y26-680
Changes proposed in this pull request
Optional automated buffer addition for cherrypicking