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 type hints #64

Merged
merged 19 commits into from Feb 29, 2024
Merged

Add type hints #64

merged 19 commits into from Feb 29, 2024

Conversation

jodal
Copy link
Member

@jodal jodal commented Feb 25, 2024

I don't really expect a full review of this, but feel free to look around and
maybe check the branch out and try browsing the code in an IDE with good type
support, like VS Code with the Pylance extension with "Inlay Hints" enabled.

  • Require Mopidy 4.0
  • Make ruff enforce the presence of type hints
  • Add type hints to everything
  • Set up pyright in basic checking mode
  • Fix pyright's basic warnings
  • Move MpdContext out of dispatcher module
  • Move util functions out of MpdDispatcher class
  • Expose MpdUriMapper directly to protocol implementation
  • Move subsystem events/subscriptions to MpdDispatcher
  • Make MpdDispatcher use MpdSession directly
  • Replace regexp pattern with full string matching
  • Replace protocol_kwargs with something more type safe
  • Properly type config object with MPD config
  • Refactor command handler registration
  • Work around too flexible typing of collections in models
  • Work around lost type information
  • Increase pyright's type checking mode to standard

@jodal jodal force-pushed the type-hints branch 2 times, most recently from 98eec8e to 4c59a07 Compare February 26, 2024 00:08
src/mopidy_mpd/dispatcher.py Outdated Show resolved Hide resolved
src/mopidy_mpd/dispatcher.py Outdated Show resolved Hide resolved
src/mopidy_mpd/dispatcher.py Outdated Show resolved Hide resolved
src/mopidy_mpd/dispatcher.py Show resolved Hide resolved
src/mopidy_mpd/dispatcher.py Outdated Show resolved Hide resolved
src/mopidy_mpd/translator.py Outdated Show resolved Hide resolved
src/mopidy_mpd/translator.py Show resolved Hide resolved
src/mopidy_mpd/translator.py Show resolved Hide resolved
src/mopidy_mpd/translator.py Show resolved Hide resolved
src/mopidy_mpd/translator.py Show resolved Hide resolved
@kingosticks
Copy link
Member

I had a good go at a review. It all works fine as far as I can tell.

@jodal
Copy link
Member Author

jodal commented Feb 29, 2024

@kingosticks I've responded to or addressed all your comments now. Want to have another look?

@kingosticks
Copy link
Member

LGTM

@jodal jodal merged commit 133f381 into mopidy:main Feb 29, 2024
5 of 7 checks passed
@jodal jodal deleted the type-hints branch February 29, 2024 22:44
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.

None yet

2 participants