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

Use config_entry.unique_id in Linky #31051

Merged
merged 3 commits into from
Jan 27, 2020

Conversation

Quentame
Copy link
Member

Breaking Change:

None

Description:

Use config_entry.unique_id in Linky

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

def _show_setup_form(self, user_input=None, errors=None):
pass

async def _show_setup_form(self, user_input=None, errors=None):
Copy link
Member

Choose a reason for hiding this comment

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

Why make it async ?

Copy link
Member Author

@Quentame Quentame Jan 27, 2020

Choose a reason for hiding this comment

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

It's async in adguard, icloud, twentemilieu and also not a callback (but not async) in elgato and wled.

Don't know exactly which is the best, but can fix other components if needed.

  • No callback no async
  • No callback async
  • Callback no async
  • Callback async

Getting confused, it's not homogen in the code 😅

Copy link
Member

Choose a reason for hiding this comment

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

It's best to not define coroutines if we don't need to. There's an overhead to awaiting a coroutine compared to running a callback in the event loop.

@MartinHjelmare MartinHjelmare moved this from Needs review to Review in progress in Dev Jan 23, 2020
Copy link
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!

Dev automation moved this from Review in progress to Reviewer approved Jan 27, 2020
@balloob balloob merged commit a73a1a4 into home-assistant:dev Jan 27, 2020
Dev automation moved this from Reviewer approved to Done Jan 27, 2020
@Quentame Quentame deleted the linky/config_unique_id branch January 27, 2020 20:39
@lock lock bot locked and limited conversation to collaborators Jan 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Dev
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants