-
Notifications
You must be signed in to change notification settings - Fork 5
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
Training rigs system test #44
Training rigs system test #44
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #44 +/- ##
==========================================
- Coverage 33.79% 33.78% -0.01%
==========================================
Files 16 16
Lines 364 367 +3
==========================================
+ Hits 123 124 +1
- Misses 241 243 +2 ☔ View full report in Codecov by Sentry. |
src/htss_rig_bluesky/devices.py
Outdated
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.
This file is now obsolete thanks to https://github.com/DiamondLightSource/dodal/blob/main/src/dodal/beamlines/training_rig.py so you can just delete it
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.
These devices are being used in multiple places which require refactoring multiple plans I suggest we create a new issue to upgrade the plans to the latest version of ophyd-async
I have create a issue for it #46
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, just a couple of things
"ophyd", | ||
"ophyd-async==0.4.0", | ||
"ophyd-async>=0.8.0", |
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.
Should: If you delete the device you should be able to completely remove the dependency on ophyd/ophyd-async
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.
Out of scope but could you make an issue to include these plans in the blueapi installation and write system tests for them?
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.
I have added a issue here for this #46
I can add that the plans should be moved as well for it
1fbca17
to
b9e983a
Compare
No description provided.