-
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
refactor(robot-server): consolidate runs into a single Run model #8685
Conversation
Codecov Report
@@ Coverage Diff @@
## edge #8685 +/- ##
==========================================
- Coverage 75.23% 75.22% -0.02%
==========================================
Files 1207 1207
Lines 37959 37934 -25
Branches 2792 2792
==========================================
- Hits 28559 28535 -24
+ Misses 8999 8998 -1
Partials 401 401
Flags with carried forward coverage won't be shown. Click here to find out more.
|
commands: List[RunCommandSummary] = Field( | ||
..., | ||
description="Unique identifier of the protocol this run will execute.", | ||
description="Protocol commands queued, running, or executed for the run.", | ||
) |
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.
Is the idea for this single commands
list to contain both LPC commands and protocol commands?
If so, I guess it would take annotations
to separate LPC commands from the actual protocol run log. Since we don't have that yet, is App+UI fine with not having that ability, for 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.
Yes I think we're fine with that ability, because we will be using the protocol analysis commands as the scaffolding for the run log. We shouldn't need to use annotations to start the run log at the right command.
Overview
As discussed in Slack, this is a simplification PR to remove the unnecessary distinction between a BasicRun and a ProtocolRun.
Changelog
Run
, which may optionally point to aprotocolId
if it's there to run a protocolReview requests
Code changes make sense. This back-end PR requires a corresponding front-end PR
Risk assessment
Low