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

Pass hass_config to load_platform #17952

Merged
merged 3 commits into from Oct 29, 2018
Merged

Pass hass_config to load_platform #17952

merged 3 commits into from Oct 29, 2018

Conversation

balloob
Copy link
Member

@balloob balloob commented Oct 29, 2018

Description:

Follow up on #17942

Fixes #17935

Found a lot more. Made the parameters mandatory and also added a safety assert in case people just add {}.

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@balloob balloob requested a review from a team as a code owner October 29, 2018 14:15
@ghost ghost assigned balloob Oct 29, 2018
@ghost ghost added the in progress label Oct 29, 2018
Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'

@balloob balloob merged commit 6ae345b into dev Oct 29, 2018
@balloob balloob deleted the guard-discovery-no-config branch October 29, 2018 18:21
@ghost ghost removed the in progress label Oct 29, 2018
balloob added a commit that referenced this pull request Oct 29, 2018
* Pass hass_config to load_platform

* Fix tests

* Lint
@balloob balloob mentioned this pull request Oct 29, 2018
mvn23 added a commit to mvn23/home-assistant that referenced this pull request Oct 30, 2018
claytonjn added a commit to claytonjn/home-assistant that referenced this pull request Oct 31, 2018
@bratanon
Copy link
Contributor

bratanon commented Nov 1, 2018

Why on earth is this not under the list of breaking changes? It clearly breaks almost all custom components.

@balloob
Copy link
Member Author

balloob commented Nov 2, 2018

Then almost all custom components were breaking Home Assistant 🙈

But seriously, you're the first to complain about this. So I don't think your assumption that it breaks all is right. A PR to add this to the breaking changes in the release notes is welcome.

@lostage
Copy link

lostage commented Nov 2, 2018

He's not the first...

#18003 (comment)

It looks like this was broken by #17952 which just was merged in yesterday. The hass_config parameter was made required.

@balloob do you have any insight into this change and why it was made mid-release?

@bratanon
Copy link
Contributor

bratanon commented Nov 2, 2018

Well, its an API change of a well used method. Not sure how this is handled with HA tho.

This should in my opinion be a major version bump since it is not backward compatible and breaks "many" plugins, even plugins in HA core.

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

Successfully merging this pull request may close these issues.

None yet

6 participants