Skip to content

Conversation

@tpoliaw
Copy link
Contributor

@tpoliaw tpoliaw commented Sep 12, 2025

Moving the numtracker into the bluesky context means we don't need to do the 'create context -> create numtracker -> configure context -> check path provider` shenanigans in the top level setup function and the path provider can be available to pass to device factories if we move to a dodal-less approach to devices.

@tpoliaw tpoliaw requested a review from a team as a code owner September 12, 2025 11:13
@codecov
Copy link

codecov bot commented Sep 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.60%. Comparing base (0447ea1) to head (cc8896a).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1200      +/-   ##
==========================================
+ Coverage   94.59%   94.60%   +0.01%     
==========================================
  Files          41       41              
  Lines        2588     2578      -10     
==========================================
- Hits         2448     2439       -9     
+ Misses        140      139       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@callumforrester
Copy link
Contributor

@tpoliaw while I like the reduction in shenanigans I was already worried about how much of a god object BlueskyContext was becoming. I don't think that should block merge but it might be useful to have some thoughts (and make a ticket) about breaking it up

@tpoliaw
Copy link
Contributor Author

tpoliaw commented Sep 16, 2025

I don't know how much of a god object it is. As far as I can see it's a bucket holding all the bluesky state. If the device loading is moved into dodal, everything else makes sense to be left, doesn't it?

If anything it's cleaner than using a module full of globals as a state object.

Copy link
Contributor

@callumforrester callumforrester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, definitely an improvement

@tpoliaw tpoliaw changed the title Move numtracker into the BlueskyContext refactor: Move numtracker into the BlueskyContext Sep 16, 2025
@tpoliaw tpoliaw merged commit db8d490 into main Sep 23, 2025
19 checks passed
@tpoliaw tpoliaw deleted the interface_context branch September 23, 2025 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants