-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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
Area units and conversion between metric and US #123563
Area units and conversion between metric and US #123563
Conversation
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.
There is a merge conflict.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
@mikey0000 CI checks are failing. Please fix that, and then make sure to click the "Ready for review" button to have your PR reviewed. |
@mikey0000 I don't see the rationale for adding the area converter. Which integrations will make use of this? I think there needs to be an approved architecture discussion first. If that's already the case, please link to it. |
All lawn mower integrations use area squared, added the architecture discussion link |
Thanks @mikey0000 for adding the link to the architecture proposal 👍 However, just adding a utility function for converting areas is not enough for integrations - and users - to benefit. You'll also need to add a new sensor device class, and add preferred area units to the metric and us customary unit systems. Here's an example of a more complete PR, although it did not add preferred conductivity units to the unit systems: #118728 |
Thanks, I'll update it. Thanks also for the reply on the Architecture discussion. |
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@mikey0000 the architecture proposal is approved 👍 |
Thanks I really appreciate the support, see you in the code owners channel 😄 |
Marked the PR as draft as there is quite a bit of conflicts that needs to be sorted. |
"tsconfig-paths-webpack-plugin": "^4.1.0",
Sorted |
Not sure why one test is timing out. |
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.
Nice, thanks @mikey0000 👍
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.
LGTM 👍
Please open and link to documentation PRs for: |
added |
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.
Some conflicts appeared, could you have a look?
Breaking change
Proposed change
Add Area units and conversions from metric to US customary
home-assistant/architecture#1124
Type of change
Additional information
This PR fixes the lack of conversion of area squared
developer docs: Add blog and update number and sensor entity docs developers.home-assistant#2448
Checklist
ruff format 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
.To help with the load of incoming pull requests: