Skip to content

refactor(logitech_receiver/hidppx0_constants.py): replace ERROR NamedInt by IntEnum #2645

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

Merged
merged 25 commits into from
Nov 2, 2024

Conversation

rloutrel
Copy link
Contributor

Also did a small refactoring in run, for readibility (and reduce duplicated code)

rloutrel and others added 14 commits October 20, 2024 23:11
…Error and BoltPairingError (Fix pre-commit checks)
…ion before replacing ERRORNamedInts by IntEnum
…nction before replacing ERROR NamedInts by IntEnum
…ion before replacing ERRORNamedInts by IntEnum (add exclusion for macOS)
…ion before replacing ERRORNamedInts by IntEnum (fix for python < 3.10)
…fore replacing ERROR NamedInts by IntEnum (focusing on the call order when receiving errors)
…e short and long registers in a single loop structure for improved readability and reduced code duplication.
… and hidpp20 errors in the code for readibility.
…Int by IntEnum. (fix problem with | operator when typing with python 3.8)
@rloutrel rloutrel changed the title refactor(logitech_receiver/hidpp10_constants.py): replace ERROR NamedInt by IntEnum refactor(logitech_receiver/hidppx0_constants.py): replace ERROR NamedInt by IntEnum Oct 26, 2024
@MattHag MattHag requested a review from pfps November 1, 2024 17:23
@rloutrel
Copy link
Contributor Author

rloutrel commented Nov 2, 2024

Is Ok. I am on this problem...

@rloutrel
Copy link
Contributor Author

rloutrel commented Nov 2, 2024

Ready for merge. I will stop on re-factoring to try the implementation of something like a configuration (and menu in app) if you agree and think it is a good idea.

Copy link
Collaborator

@pfps pfps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@pfps pfps merged commit a19461b into pwr-Solaar:master Nov 2, 2024
5 checks passed
@MattHag
Copy link
Collaborator

MattHag commented Nov 2, 2024

Ready for merge. I will stop on re-factoring to try the implementation of something like a configuration (and menu in app) if you agree and think it is a good idea.

I wouldn't recommend it, as the existing code needs so much clean up and improvement, that it's better to do that first. Otherwise it is likely that the mess grows and grows. We should aim for further restructuring into a well testable business logic, that is not directly dependent on files or the user interface, as proposed in "Architecture Patterns with Python: Enabling Test-Driven Development, Domain-Driven Design, and Event-Driven Microservices".

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.

3 participants