-
Notifications
You must be signed in to change notification settings - Fork 183
refactor(api, shared-data): update stacker commands to unsafe #18157
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
Conversation
8f291dd
to
e82411f
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## edge #18157 +/- ##
==========================================
+ Coverage 21.53% 23.46% +1.92%
==========================================
Files 3038 3038
Lines 254518 255762 +1244
Branches 25662 24198 -1464
==========================================
+ Hits 54813 60012 +5199
+ Misses 199692 195735 -3957
- Partials 13 15 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
ee73489
to
b82a9fd
Compare
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.
Thanks!
Re schema 12: I think it's reasonable to leave it as-is, but you could also delete these commands from it, since it sounds like we consider these commands to have not been stable and usable back then.
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.
If we're calling these commands unsafe, I'd appreciate docstrings somewhere that explain why they're unsafe. Like, from the perspective of someone sending commands to the robot, what are the caveats about these, why shouldn't these be used in protocols, and what are the preconditions/postconditions to use these properly.
Overview
Sets the
flexStacker/prepareShuttle
,closeLatch
andopenLatch
commands as unsafe commands.We don't have to update schema 12 too, right?