-
Notifications
You must be signed in to change notification settings - Fork 452
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
Allow optional dependencies between components #6291
Allow optional dependencies between components #6291
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine to merge as-is.
Some suggestions for improvement (not critical/nitpicks):
- Refactor out duplicate code in tests into common functions to reduce duplication.
- Get rid of the
loop
fixture in these tests. If the fixture is not used (as reported Pylint) it has some hidden side effects that are needed to configure this test (or alternatively: it does nothing). This implies it should've been set somewhere higher up in the fixture hierarchy.
7a86309
to
a1b2505
Compare
@qstokkink Thanks! I'll refactor the tests code a bit later to remove duplication. |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
This PR adds an important possibility for components to have optional dependencies.
Basically, it means that a component B can declare: "I don't need to have component A enabled, but if it is enabled, it should be activated before me".
Optional component dependencies are necessary to fix various scripts like
run_tunnel_helper.py
, as not all components are used in such scripts.