Display unit of elevation in met config flow#88283
Conversation
|
Hey there @Danielhiversen, @thimic, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
df23178 to
ef32efb
Compare
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
ef32efb to
2cd6318
Compare
emontnemery
left a comment
There was a problem hiding this comment.
LGTM based on the explanation from @chrisx8 👍
|
@emontnemery Sorry it isn't, it removes a conversion, thus breaking existing installations. |
frenck
left a comment
There was a problem hiding this comment.
Left a comment above, this PR needs changes to prevent is being a breaking change for existing installations.
../Frenck
|
@chrisx8 it's quite the mess you've uncovered here. I think we need to do this in steps, where one step is one PR:
|
2cd6318 to
9dced4c
Compare
|
Rebased and added a breaking change note about requiring elevation to be in meters |
9dced4c to
3ac9144
Compare
Co-authored-by: lijake8 <lijake8@users.noreply.github.com> Signed-off-by: Chris Xiao <30990835+chrisx8@users.noreply.github.com>
3ac9144 to
56e7b93
Compare
56e7b93 to
c7d0147
Compare
c7d0147 to
e7fb048
Compare
|
@emontnemery Any update here? This PR requires elevation in |
emontnemery
left a comment
There was a problem hiding this comment.
Thanks a lot @chrisx8 for sorting out this mess 👍
A breaking change note has been added, and the impact of the change is now minimal because only manually setup config entries are affected
Breaking change
The
metintegration now requires elevation to be set in meters, regardless of the unit system set in Home Assistant. This mirrors the behavior of Home Assistant Core, which requires elevation to be in meters regardless of the unit system.This only affects users who:
metintegration manually via Settings (not set up automatically during onboarding), ANDPlease double-check that elevation in
metis set correctly, using the integration's "Configure" dialog.Proposed change
Require elevation in
metto be in meters.Display the unit of elevation (always meters) in
met's config flow data entry dialog.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: