-
-
Notifications
You must be signed in to change notification settings - Fork 187
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
Improve categorization of features #904
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #904 +/- ##
==========================================
- Coverage 90.40% 90.39% -0.01%
==========================================
Files 70 70
Lines 4941 4937 -4
Branches 1220 1219 -1
==========================================
- Hits 4467 4463 -4
Misses 384 384
Partials 90 90 ☔ View full report in Codecov by Sentry. |
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.
Looks good, there's a lot of changes across files here so I think we should get this merged asap to avoid merge conflicts. Two questions we can address subsequently are:
- Should we make category a required parameter that must be explicitly set. It's being set on so many features now this shouldn't be too onerous.
- Should we have some mechanism for indicating the type of data in sensors, e.g. timestamps, so we can derive more sensor classes in HA. Maybe this could be added to unit as
seconds
etc.
Yeah, we should probably require explicit setting for category and type, and remove the magic from
Worth considering, but it probably should probably be explicit and separate from the |
This will update the categorization of features as discussed in the now-obsolete #879:
The
id
is also made mandatory for features, which will allow using it as a translation key without fear of breaking the "API" inadvertently.