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

Add linter to enforce import rules #840

Open
DominicOram opened this issue Oct 14, 2024 · 6 comments
Open

Add linter to enforce import rules #840

DominicOram opened this issue Oct 14, 2024 · 6 comments

Comments

@DominicOram
Copy link
Contributor

DominicOram commented Oct 14, 2024

In mx-bluesky we have enforced some import rules with a linter DiamondLightSource/mx-bluesky#568. We could do the same with dodal. The rules will probably be at least:

  • common doesn't import from devices
  • devices.ixx doesn't import from devices.iyy

Acceptance Criteria

  • We implement rules discussed on this issue
  • Accompanying docs are written if the rules are not clear
@callumforrester
Copy link
Contributor

How about "nothing within dodal imports from beamlines, excluding tests"? beamlines is essentially just configuration and I don't think anything should depend on it.

@DiamondJoseph
Copy link
Contributor

Having just stumbled into making a circular dependency: love it.

@rtuck99
Copy link
Contributor

rtuck99 commented Oct 15, 2024

Also plans can depend on devices but not the other way around

@stan-dot
Copy link
Contributor

How about "nothing within dodal imports from beamlines, excluding tests"? beamlines is essentially just configuration and I don't think anything should depend on it.

what if we make 'beamlines' just YAML? 👀

@callumforrester
Copy link
Contributor

That possibility was discussed at the bluesky workshop at NOBUGS. I think if we went that route we would be trying to find a multi-facility solution for loading devices. Currently everyone has rolled their own using either databases, config files, or code-as-config (like dodal).

@DominicOram
Copy link
Contributor Author

Given the feedback on this I think we can say that it's been agreed upon that we do it. Updating acceptance criteria and pulling into next hyperion sprint

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo This Sprint
Development

No branches or pull requests

5 participants