Skip to content

Bump pydaikin 2.10.5#95656

Merged
bdraco merged 11 commits into
home-assistant:devfrom
mover85:update-pydaikin
Jul 5, 2023
Merged

Bump pydaikin 2.10.5#95656
bdraco merged 11 commits into
home-assistant:devfrom
mover85:update-pydaikin

Conversation

@mover85
Copy link
Copy Markdown
Contributor

@mover85 mover85 commented Jul 1, 2023

Proposed change

Bump Daikin version, changelog: https://bitbucket.org/mustang51/pydaikin/branches/compare/v2.10.5%0Dv2.9.0

Add zone temperature control.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

@home-assistant
Copy link
Copy Markdown
Contributor

home-assistant Bot commented Jul 1, 2023

Hey there @fredrike, mind taking a look at this pull request as it has been labeled with an integration (daikin) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of daikin can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign daikin Removes the current integration label and assignees on the pull request, add the integration domain after the command.

Copy link
Copy Markdown
Contributor

@fredrike fredrike left a comment

Choose a reason for hiding this comment

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

LGTM

bdraco
bdraco previously approved these changes Jul 3, 2023
@bdraco bdraco modified the milestone: 2023.7.0 Jul 3, 2023
@bdraco bdraco dismissed their stale review July 3, 2023 03:37

The unique id migration is missing tests

@bdraco
Copy link
Copy Markdown
Member

bdraco commented Jul 3, 2023

Just realized there are no tests for the unique id migration. We should have at least a basic test for this as these tend to be hard to fix later if something goes wrong

@mover85 mover85 requested a review from bdraco July 4, 2023 02:20
Comment thread tests/components/daikin/test_init.py Outdated
Comment thread tests/components/daikin/test_init.py
@mover85 mover85 requested a review from bdraco July 4, 2023 11:23
Copy link
Copy Markdown
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

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

Nice! Looks like there is a stale docstring to fix

Comment thread tests/components/daikin/test_init.py Outdated
@bdraco
Copy link
Copy Markdown
Member

bdraco commented Jul 4, 2023

Looks good 👍

It would be nice to cover that one line before merging

@mover85
Copy link
Copy Markdown
Contributor Author

mover85 commented Jul 5, 2023

It would be nice to cover that one line before merging

I'm not sure if I can cover that line, should I unnest the function to allow testing?

@frenck frenck added the smash Indicator this PR is close to finish for merging or closing label Jul 5, 2023
Comment thread homeassistant/components/daikin/__init__.py Outdated
Comment thread homeassistant/components/daikin/__init__.py Outdated
Comment thread homeassistant/components/daikin/__init__.py Outdated
Comment thread homeassistant/components/daikin/__init__.py Outdated
@home-assistant
Copy link
Copy Markdown
Contributor

home-assistant Bot commented Jul 5, 2023

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

@home-assistant home-assistant Bot marked this pull request as draft July 5, 2023 09:43
@mover85 mover85 marked this pull request as ready for review July 5, 2023 10:57
@home-assistant home-assistant Bot requested a review from MartinHjelmare July 5, 2023 10:57
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare 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!

Copy link
Copy Markdown
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

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

Thanks @mover85 for improving the integration 🎉

@bdraco bdraco merged commit f028d1a into home-assistant:dev Jul 5, 2023
@jypsilantis
Copy link
Copy Markdown

Just wondering if this made it into Core 2023.7.0. I just tried and the 4/3 parameter issue is still there. Happy to assist with logs etc if anyone needs them.

@MartinHjelmare
Copy link
Copy Markdown
Member

No this wasn't tagged for the release.

@mover85 @fredrike is this safe to go in a patch release?

joostlek pushed a commit to joostlek/core that referenced this pull request Jul 6, 2023
@mover85
Copy link
Copy Markdown
Contributor Author

mover85 commented Jul 7, 2023

@mover85 @fredrike is this safe to go in a patch release?

Yes

@MartinHjelmare MartinHjelmare added this to the 2023.7.2 milestone Jul 7, 2023
@MartinHjelmare
Copy link
Copy Markdown
Member

I've tagged the PR for the next patch release.

@github-actions github-actions Bot locked and limited conversation to collaborators Jul 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cherry-picked cla-signed code-owner-approved dependency Pull requests marked as a dependency upgrade integration: daikin Quality Scale: platinum smash Indicator this PR is close to finish for merging or closing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Daikin breaking changes - renaming devices

7 participants