search

DiamondLightSource / mx-bluesky

Bluesky plans, plan stubs, and utilities for MX beamlines
https://diamondlightsource.github.io/mx-bluesky/
Apache License 2.0
0 stars 2 forks source link

Move XRC callbacks to common #652

Open olliesilvester opened 1 week ago

olliesilvester commented 1 week ago

Fixes https://github.com/DiamondLightSource/mx-bluesky/issues/214 and https://github.com/DiamondLightSource/mx-bluesky/issues/215

Moves lots of the external_interaction code into common. Some main bits:

Probably difficult to review, sorry

codecov[bot] commented 1 week ago

Codecov Report

Attention: Patch coverage is 97.83550% with 5 lines in your changes missing coverage. Please review.

Project coverage is 84.98%. Comparing base (9aa3698) to head (590b6c6).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #652 +/- ## ========================================== - Coverage 85.00% 84.98% -0.02% ========================================== Files 98 96 -2 Lines 6548 6546 -2 ========================================== - Hits 5566 5563 -3 - Misses 982 983 +1 ``` | [Components](https://app.codecov.io/gh/DiamondLightSource/mx-bluesky/pull/652/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DiamondLightSource) | Coverage Δ | | |---|---|---| | [i24 SSX](https://app.codecov.io/gh/DiamondLightSource/mx-bluesky/pull/652/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DiamondLightSource) | `69.59% <ø> (ø)` | | | [hyperion](https://app.codecov.io/gh/DiamondLightSource/mx-bluesky/pull/652/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DiamondLightSource) | `96.23% <100.00%> (-0.56%)` | :arrow_down: | | [other](https://app.codecov.io/gh/DiamondLightSource/mx-bluesky/pull/652/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DiamondLightSource) | `96.97% <96.81%> (+1.99%)` | :arrow_up: |
olliesilvester commented 1 week ago

See https://github.com/DiamondLightSource/mx-bluesky/issues/655 for sorting out the tests. Annoyingly, we might want to do this before merging, so we can be sure that this PR didn't break anything, but I don't want to hold up @noemifrisina 's work

noemifrisina commented 1 week ago

See #655 for sorting out the tests. Annoyingly, we might want to do this before merging, so we can be sure that this PR didn't break anything, but I don't want to hold up @noemifrisina 's work

If all the important bits have been moved and sorting out the tests is all that's left to this one, I think it would be worth it to sort out the tests before merging this. While that might take awhile, as you said at least we'd be reasonably sure things are still working after this PR. For my work, I can just make a new branch off this one in the meantime and make a start

olliesilvester commented 1 week ago

See #655 for sorting out the tests. Annoyingly, we might want to do this before merging, so we can be sure that this PR didn't break anything, but I don't want to hold up @noemifrisina 's work

If all the important bits have been moved and sorting out the tests is all that's left to this one, I think it would be worth it to sort out the tests before merging this. While that might take awhile, as you said at least we'd be reasonably sure things are still working after this PR. For my work, I can just make a new branch off this one in the meantime and make a start

Cool, as long as it's not holding you up

noemifrisina commented 1 week ago

On a first go through, looks okay, thank you. Will wait to finish review and approve till you're done with the tests. I'd say the fact that they pass right now is a good start though :)