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

Use "kB" and "s" as UPnP/IGD units #29552

Merged
merged 1 commit into from
Dec 6, 2019
Merged

Use "kB" and "s" as UPnP/IGD units #29552

merged 1 commit into from
Dec 6, 2019

Conversation

scop
Copy link
Member

@scop scop commented Dec 6, 2019

Breaking Change:

UPnP/IGD units of measurement have been aligned with other integrations and common uses, they're now kB and kB/s instead of kbyte and kbyte/sec respectively.

Description:

For consistency with various existing components, and they're more
commonly used and compact than "kbyte" and "sec".

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

If the code does not interact with devices:

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

For consistency with various existing components, and they're more
commonly used and compact than "kbyte" and "sec".
@probot-home-assistant
Copy link

Hey there @robbiet480, mind taking a look at this pull request as its been labeled with a integration (upnp) you are listed as a codeowner for? Thanks!

@dgomes
Copy link
Contributor

dgomes commented Dec 6, 2019

This is a breaking change for people who keep long records...

@MartinHjelmare
Copy link
Member

Please add a breaking change paragraph in the PR description.

@scop
Copy link
Member Author

scop commented Dec 6, 2019

Added. I'm not sure about the extent of the breakage, so didn't try to elaborate on that.

Also, have there been changes like this in which previous measurements have been migrated? Should be doable in the states table although maybe a little heavy, and wouldn't affect external databases such as InfluxDB anyway.

@fabaff fabaff merged commit d257fff into home-assistant:dev Dec 6, 2019
@scop scop deleted the upnp-units branch December 6, 2019 18:33
@MartinHjelmare
Copy link
Member

I don't know if we have ever done migration of state history in the database. We have migrated the database but only in terms of layout I think.

@lock lock bot locked and limited conversation to collaborators Dec 7, 2019
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.

6 participants