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

Fix the default value for the notification fallback callback #117

Merged
merged 1 commit into from
Sep 12, 2022

Conversation

Flameeyes
Copy link
Contributor

After #112 the None value is no longer a valid fallback_callback
value. Instead of allowing it, provide a default no-op callback that
does nothing and use that as default.

This was surfacing as home-assistant/core#77730.

@rytilahti rytilahti changed the title Fix the default value for the notification fallback callback. Fix the default value for the notification fallback callback Sep 11, 2022
After rytilahti#112 the `None` value is no longer handled as a valid
`fallback_callback`.

This change reintroduce support for it, by considering it the same as
no callback given for the notification.

This was surfacing as home-assistant/core#77730.
Copy link
Owner

@rytilahti rytilahti left a comment

Choose a reason for hiding this comment

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

Looks much simpler, thanks! 👍

@rytilahti rytilahti added the bug label Sep 12, 2022
@rytilahti rytilahti merged commit 03d90db into rytilahti:master Sep 12, 2022
rytilahti added a commit that referenced this pull request Sep 12, 2022
[Full Changelog](0.15...0.15.1)

**Fixed bugs:**

- Fix the default value for the notification fallback callback [\#117](#117) (@Flameeyes)
@rytilahti rytilahti mentioned this pull request Sep 12, 2022
@rytilahti
Copy link
Owner

rytilahti commented Sep 12, 2022

Released 0.15.1: https://pypi.org/project/python-songpal/0.15.1/ - feel free to create a bump PR to homeassistant!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants