Skip to content

refactor(shared-data, protocol-designer, app): Update moveToWell command copy #18776

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

Open
wants to merge 4 commits into
base: chore_release-8.5.0
Choose a base branch
from

Conversation

mjhuff
Copy link
Contributor

@mjhuff mjhuff commented Jun 27, 2025

Closes AUTH-1777

Overview

Updates the moveToWell command text so the run log is a bit less ambiguous about command output.

It appears that the offset and origin are always provided for this command, based on looking at a few files in the engine stemming from here.

NOTE: I had to revert 9e96e01 given the extensive (74) type errors that would need fixing, and I don't have the time to address them right now. I'll leave a TODO and just default to any empty string for now. It should be sufficient if these values are always present and makes this change pretty much riskless, which seems nice given where we are in the QA process.

Current Behavior

Screenshot 2025-06-27 at 7 14 43 PM

Fixed Behavior

Screenshot 2025-06-27 at 7 13 11 PM

Test Plan and Hands on Testing

  • See images. See ticket for protocol used to test.

Changelog

  • Updated the run log command text for the moveToWellCommand.

Risk assessment

low, just copy. Only risk comes from the assumption that we always have access to these params, but at worst we just show some undefined copy values.

mjhuff added 2 commits June 27, 2025 19:02
Looks like this is always provided in the engine, so we should probably update shared data
@mjhuff mjhuff requested review from sanni-t, ncdiehl11, jerader and a team June 27, 2025 23:19
@mjhuff mjhuff requested review from a team as code owners June 27, 2025 23:19
@mjhuff
Copy link
Contributor Author

mjhuff commented Jun 27, 2025

I'm out next week, so please merge this when it gets approval!

Copy link

codecov bot commented Jun 27, 2025

Codecov Report

Attention: Patch coverage is 0% with 18 lines in your changes missing coverage. Please review.

Project coverage is 54.64%. Comparing base (e7bc194) to head (8f5e32b).

Files with missing lines Patch % Lines
...ring/utils/commandText/getMoveToWellCommandText.ts 0.00% 18 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                   Coverage Diff                   @@
##           chore_release-8.5.0   #18776      +/-   ##
=======================================================
- Coverage                54.65%   54.64%   -0.01%     
=======================================================
  Files                     3258     3258              
  Lines                   281773   281788      +15     
  Branches                 29494    29494              
=======================================================
- Hits                    153998   153995       -3     
- Misses                  127585   127603      +18     
  Partials                   190      190              
Flag Coverage Δ
protocol-designer 18.70% <0.00%> (-0.01%) ⬇️
step-generation 4.33% <0.00%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
...ring/utils/commandText/getMoveToWellCommandText.ts 52.83% <0.00%> (-28.75%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

2 participants