Skip to content
This repository has been archived by the owner on Sep 2, 2024. It is now read-only.

Ensure proper use of class variables #814

Merged
merged 5 commits into from
Aug 3, 2023

Conversation

DominicOram
Copy link
Collaborator

Fixes #810

Link to dodal PR (if required): DiamondLightSource/dodal#123

To test:

  1. Confirm class variables are removed and tests still pass
  2. Confirm exceptions are listed in the issue

Copy link
Collaborator

@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.

Nit: FGSComposite isn't really a singleton, we just only create it once, there's no enforcement for that. All the objects it holds are singletons though. Anyway, removing the class variables from FGSComposite doesn't change anything (they are all overwritten with instance vars on init) so you might as well remove those too if you feel like it

@DominicOram
Copy link
Collaborator Author

Nit: FGSComposite isn't really a singleton, we just only create it once, there's no enforcement for that. All the objects it holds are singletons though. Anyway, removing the class variables from FGSComposite doesn't change anything (they are all overwritten with instance vars on init) so you might as well remove those too if you feel like it

Yh, I think I meant that all the devices under it are singletons anyway so it's effectively a singleton. But I've changed it now anyway, thanks.

Tests are failing due to #847

@DominicOram DominicOram merged commit 4ee4e97 into main Aug 3, 2023
4 checks passed
@DominicOram DominicOram deleted the 810_ensure_proper_use_of_class_variables branch August 3, 2023 10:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ensure we're not holding state in class variables
2 participants