Skip to content
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

Replace calls to deprecated inject() with direct factory calls #587

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

rtuck99
Copy link
Contributor

@rtuck99 rtuck99 commented Oct 21, 2024

This fixes the unit tests after the deprecation of inject()
see DiamondLightSource/dodal#845
merged in PR
DiamondLightSource/dodal#854

Copy link

codecov bot commented Oct 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.22%. Comparing base (efbe991) to head (15fbb15).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #587      +/-   ##
==========================================
- Coverage   78.22%   78.22%   -0.01%     
==========================================
  Files          92       92              
  Lines        6801     6800       -1     
==========================================
- Hits         5320     5319       -1     
  Misses       1481     1481              
Components Coverage Δ
i24 SSX 57.15% <100.00%> (-0.02%) ⬇️
hyperion 96.24% <ø> (ø)
other 100.00% <100.00%> (ø)

@rtuck99 rtuck99 changed the base branch from pin_scanspec_to_0_7_2 to main October 21, 2024 09:29
Copy link
Contributor

@DominicOram DominicOram left a comment

Choose a reason for hiding this comment

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

I'm not sure if this is all we need to do. If we don't wait for connection then is there not a danger that if we immediately try and use the device now we will fail? On the other hand waiting for connection inside the default args I think means that we will be doing work on import?

Copy link
Contributor

@dperl-dls dperl-dls left a comment

Choose a reason for hiding this comment

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

LGTM

We should revisit this later when we see what the issues are

@dperl-dls
Copy link
Contributor

@DominicOram I think the idea is that BlueApi has connected to all the devices before

@rtuck99 rtuck99 merged commit e7827d7 into main Oct 21, 2024
21 checks passed
@rtuck99 rtuck99 deleted the replace_inject branch October 21, 2024 09:45
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.

3 participants