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
Reverse component path #104087
Reverse component path #104087
Conversation
Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
Hey there @robbiet480, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
Hey there @home-assistant/core, @pvizeli, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
9a14e26
to
a7abab2
Compare
Some custom integrations published on HACS reference
A quick review of the references suggest none of the custom integrations will be affected by this change however. |
homeassistant/const.py
Outdated
@@ -16,7 +16,7 @@ | |||
REQUIRED_NEXT_PYTHON_HA_RELEASE: Final = "" | |||
|
|||
# Format for platform files | |||
PLATFORM_FORMAT: Final = "{platform}.{domain}" | |||
PLATFORM_FORMAT: Final = "{domain}.{platform}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's confusing that this is inverted now. When we talk about platforms, the domain means the domain that entities of the platform are grouped by. Eg an entity_id sensor.test_id
has the domain sensor
and belongs to a platform that has the domain sensor
by an integration test_integration
. We should stay consistent with those semantics here too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks incorrect:
core/homeassistant/components/tts/__init__.py
Lines 547 to 549 in e03ccb5
self.hass.config.components.add( | |
PLATFORM_FORMAT.format(domain=engine, platform=DOMAIN) | |
) |
The domain
parameter should be the "tts"
domain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should remove the PLATFORM_FORMAT
constant? It's only used in two places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed on discord, we should probably use PLATFORM_FORMAT
in more places instead. Let's do that in a follow-up 👍
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Proposed change
Reverse the keys in
hass.config.components
fromlight.hue
to the modernhue.light
in some remaining cases.Most cases already use
hue.light
, but some legacy code still used the old path.Rationale
This is better aligned with how platforms are actually located after home-assistant/architecture#124 / #19948
Type of change
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: