abmantis / whirlpool-sixth-sense

Whirlpool unofficial API for 6th Sense appliances
MIT License
13 stars 12 forks source link

Cleanup - type hints, remove unused imports, remove disabled auto-renewal logic, add pre-commit, etc #52

Closed NodeJSmith closed 2 months ago

NodeJSmith commented 2 months ago

Wanted to get things cleaned up a bit before working on the EventSocket logic, to make sure that PR didn't include extraneous stuff.

Cleanup includes:

Let me know if there are any concerns with these changes or if there are formatting conventions that this project uses that I may have missed.

Note: I didn't bump the version since there are no actual functionality changes and figured we wouldn't both bumping until there were new features/changes.

NodeJSmith commented 2 months ago

Only set WP-CLIENT-BRAND header on shared appliances call. Bump to 0.18.8 due to that change.

(hopefully) Fixes #https://github.com/home-assistant/core/issues/115130

@abmantis Can I get your eyes on this when you have a moment? And if you'd prefer I do the HA fix in a PR without any cleanup I can split these, just let me know

NodeJSmith commented 2 months ago

Confirmed on the HA issue that this change does fix the issue, so we'll definitely want this in as quickly as possible

abmantis commented 2 months ago

Thanks for this! Yeah, it would be great if you could split this into smaller PRs, each focused on a single change. Otherwise this will take a lot to get merged, since it will get more review rounds.

Also, it is easier to find time during the day to review small PRs :)

abmantis commented 2 months ago

It is fine to leave the version bump out as that allows us to bundle more changes if needed, and I can do it anyway for the release