sbidy / pywizlight

A python connector for WiZ devices
MIT License
463 stars 79 forks source link

General code quality fixes #92

Closed akx closed 2 years ago

akx commented 3 years ago

This PR applies a bunch of general code quality & typing fixes. In that, it may fix some future or corner-case bugs.

👉 I would strongly advise this PR not be squash-merged in case trouble arises down the line; it would make it a lot easier to bisect (not to mention git annotates look better).

In particular:

Some future improvements might include:

Of course please let me know if you want this e.g. split into multiple PRs, or if there's any concerns :)

sbidy commented 2 years ago

@akx awesome changes!! Thank you! It's a good point to refactor the PilotBuilder and PiloteParser into a clean function, but this will break eventually some integrations. But I will be more readable and usable with functions.

sbidy commented 2 years ago

@all-contributors please add @akx code

allcontributors[bot] commented 2 years ago

@sbidy

I've put up a pull request to add @akx! :tada:

akx commented 2 years ago

@akx awesome changes!! Thank you! It's a good point to refactor the PilotBuilder and PiloteParser into a clean function, but this will break eventually some integrations. But I will be more readable and usable with functions.

@sbidy Thank you!

Sure - those breaking changes could be done in the next semver-major update (0.5.x or 0.6.x maybe?).

Can you approve GH workflows for this PR (first-time contributors need approval) so I can see if there's anything that needs changes lint-wise?

akx commented 2 years ago

@sbidy I rebased this and fixed the issues that occurred in CI, but I had to edit the workflow file to do so, so you'll need to re-approve :)

akx commented 2 years ago

@sbidy Fixed the last CI fix. Once more with feeling, as they say! You can see the checks having succeeded over at https://github.com/akx/pywizlight/pull/1 ...

sbidy commented 2 years ago

Yes, hopefully I can get my hands on the PR to merge this in the next days. Thank you for the CI fix...