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

Code cleanups #57

Closed
wants to merge 2 commits into from
Closed

Code cleanups #57

wants to merge 2 commits into from

Conversation

c4rlo
Copy link
Contributor

@c4rlo c4rlo commented Nov 9, 2024

  • a4b020a addresses some minor issues I found via the ruff linter.
  • 1449745 addresses a circular dependency I noticed between the uwsm.misc and uwsm.dbus modules. This didn't seem to cause a problem in practice, but doesn't seem like a good idea. The price for fixing it is removal of a print_debug() statement in DbusInteractions.__init__(). If this is not acceptable, we can look for alternative solutions.

- Fix uwsm.misc not explicitly importing uwsm.dbus
- Remove uwsm.dbus -> uwsm.misc dependency by removing a print_debug()
@Vladimir-csp
Copy link
Owner

Thanks!
I would like to continue splitting stuff into modules (#16), so I need a way to cross-import between them. Printing/notification functions in misc require dbus functinons, and dbus functions may need to use functions from misc. There is only one print_debug() call there because I added as a test with intention to add more.

What would be the best way to achieve that? I'm open to ideas.

@c4rlo
Copy link
Contributor Author

c4rlo commented Nov 9, 2024

Options I can think of:

  1. Pull out print_debug() into its own module (e.g. debug.py) that has no dependencies, import that from everywhere else it's needed. Small downside is that it causes print_debug() to then live in a different module than the other print* functions, which is maybe a bit odd.
  2. Merge misc.py and dbus.py into one module. Goes against the goal of more modularisation.
  3. Inject the DBus dependency into the print* functions somehow, rather than having them explicitly depend on it. E.g. instead of taking a notify argument, they could get an optional notifier argument, where the caller may pass a DbusInteractions object (or None).

@c4rlo c4rlo mentioned this pull request Nov 10, 2024
@c4rlo
Copy link
Contributor Author

c4rlo commented Nov 10, 2024

I have submitted #58 as an alternative to this PR. Feel free to close this one. The modularisation discussion can continue in #16, I think.

@Vladimir-csp
Copy link
Owner

Thanks!

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.

2 participants