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
Revert "Change naming of Shelly entities to correspond with HA guidelines" #99059
Conversation
Hey there @balloob, @bieniu, @chemelli74, @bdraco, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
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 don't agree on this change. We should follow the guidelines; like every other integration.
There is no reasoning here what is broken, nor is the problem presented. At this point, this is a revert in the dark.
../Frenck
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
The problem is that currently it is broken and to fix it we will have to introduce a breaking change in the future, this is not about guidelines, we prepared this to be ready for entity translations and realized it is not helping us making entity translations as we have hardcoded parts in the entity names. I was starting to work on architecture discussion to help us solve this issue, but I see no point doing it if we don't get back to something that is working and starting clean from there. Feel free to close it if you disagree, no point putting it on requested changes. |
The problem isn't explained. Without the proper motivation there is nothing to approve, nor is there anything to request changes on. The issue isn't explained. ../Frenck |
I'm not sure why this is closed, and fail to understand it? There isn't a problem? Or there is a problem? Is the PR now closed because you don't want to elaborate on the issue? Or? 😕 ../Frenck |
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.
Thanks for the updated text and discussions @thecode!
Proposed change
Reverts #97533
Edited to explain better:
The purpose of #97533 was to align Shelly with HA guidelines and prepare for implementing entity translations. However after this PR was accepted, during testing and starting to work on entity translations we realized that:
Channel
) which there is no support for translating it right now.Channel
text) when we try to provide entity translations, which will not go in this release so the next release will also change the entity names.With the above considerations we think it would be better to delay this change until we have answers to the points above or even if we don't we try to implement all the changes in a single release so the impact on users will be minimal.
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: