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

Add Tesla custom integration #383

Merged
merged 3 commits into from
Apr 30, 2021
Merged

Conversation

alandtse
Copy link
Contributor

No description provided.

Copy link
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

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

This integration collides with a core integration. Suggestion to change its domain.

Additionally, it relies on packages that are also dependent on in the core. This may cause a breakage when dependencies get out of sync. The suggestion is to depend on those requirements using a range instead of a specific version.

@alandtse
Copy link
Contributor Author

It's meant as a replacement to core. You can't run both at the same time. By forcing an override, it makes it clear.

For the dependency, are you talking about teslajsonpy or the subdependencies under that?

@frenck
Copy link
Member

frenck commented Apr 29, 2021

It's meant as a replacement to core. You can't run both at the same time. By forcing an override, it makes it clear.

It isn't clear, we have the same issue with shelly at this moment. It must be changed for this PR to get merged in.

For the dependency, are you talking about teslajsonpy or the subdependencies under that?

testlajsonpy indeed.

@alandtse
Copy link
Contributor Author

I can change the domain but the custom component won't work with a lower version of teslajsonpy like what is in core. Or are you saying a ~0.18.0 is acceptable because the core component will still have to pin?

@frenck
Copy link
Member

frenck commented Apr 29, 2021

Yeah you could pin in a range like: teslajsonpy>=0.18.0

That said, if you have a requirement range that doesn't include the dependency of the core, it might lead to problems for end-users.

alandtse added a commit to alandtse/tesla that referenced this pull request Apr 30, 2021
This was required by home-assistant/wheels-custom-integrations#383 (comment)

Tesla in core will be non-functional regardless.
Copy link
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

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

Thanks, @alandtse 👍

../Frenck 🚂

@frenck frenck merged commit 6c14658 into home-assistant:master Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants