-
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(api,robot-server): Various small refactors #14665
Conversation
This will help in subsequent work, where we'll rearrange how internal state keeps track of the running command.
Make it agree with the docstring of the subject function, and with the actual behavior. Also fix an accidentally duplicated command ID.
Without confirming whether this behavior is actually good or correct.
This also matches `GET /runs/{id}/commands` and `GET /maintenance_runs/{id}/commands`.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## edge #14665 +/- ##
==========================================
- Coverage 67.34% 67.34% -0.01%
==========================================
Files 2485 2485
Lines 71439 71423 -16
Branches 9057 9057
==========================================
- Hits 48114 48098 -16
Misses 21173 21173
Partials 2152 2152
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
Looks good to me, good collection of fixes.
Overview
Various small refactors to prepare for PR #14664, which goes towards EXEC-301.
Test Plan
This is sufficiently covered by automated tests.
Changelog
See the commit messages.
Review requests
None in particular. It's probably easiest to review this commit-by-commit.
Risk assessment
Low.