-
-
Notifications
You must be signed in to change notification settings - Fork 36.8k
Jewish Calendar quality scale #143763
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
Jewish Calendar quality scale #143763
Changes from all commits
089dc7b
b22cba4
cd1d778
752c4e7
bd7fc9c
78145bc
953d81a
c562035
be8ed1f
84165e5
22648e0
fbe7d78
0017344
5a9e44c
b1a92c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| rules: | ||
tsvi marked this conversation as resolved.
Show resolved
Hide resolved
tsvi marked this conversation as resolved.
Show resolved
Hide resolved
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. JewishCalendarSensorDescription(
key="date",
translation_key="hebrew_date",
value_fn=lambda info: str(info.hdate),
attr_fn=lambda info: {
"hebrew_year": str(info.hdate.year),
"hebrew_month_name": str(info.hdate.month),
"hebrew_day": str(info.hdate.day),
},
),Would it make sense to create separate entities for these attributes?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No. We discussed this already. The date provides all of them in a single string.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why don't we disable them by default? I mean it sounds like it has a reason for users to automate on them
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We had this discussion here: #116252 (comment)
tsvi marked this conversation as resolved.
Show resolved
Hide resolved
tsvi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| # Bronze | ||
| action-setup: done | ||
| appropriate-polling: done | ||
tsvi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| brands: done | ||
| common-modules: done | ||
| config-flow-test-coverage: done | ||
tsvi marked this conversation as resolved.
Show resolved
Hide resolved
tsvi marked this conversation as resolved.
Show resolved
Hide resolved
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we test that the option flow refreshes the entry?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "Refreshes" meaning that the updated values are taken into consideration?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| config-flow: done | ||
| dependency-transparency: done | ||
| docs-actions: done | ||
| docs-high-level-description: done | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. See home-assistant/home-assistant.io#39998 |
||
| docs-installation-instructions: done | ||
| docs-removal-instructions: done | ||
| entity-event-setup: done | ||
| entity-unique-id: done | ||
| has-entity-name: done | ||
| runtime-data: done | ||
| test-before-configure: | ||
| status: exempt | ||
| comment: Local calculation does not require configuration. | ||
| test-before-setup: | ||
| status: exempt | ||
| comment: Local calculation does not require setup. | ||
| unique-config-entry: | ||
| status: done | ||
| comment: >- | ||
| The multiple config entry was removed due to multiple bugs in the | ||
| integration and low ROI. | ||
| We might consider revisiting this as an additional feature in the future | ||
| to allow supportong multiple languages for states, multiple locations and maybe | ||
| use it as a solution for multiple Zmanim configurations. | ||
|
|
||
| # Silver | ||
| action-exceptions: done | ||
tsvi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| config-entry-unloading: done | ||
| docs-configuration-parameters: done | ||
| docs-installation-parameters: done | ||
|
Comment on lines
+36
to
+37
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a neat table you can use to format this, now everything is their own header
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. See home-assistant/home-assistant.io#39998 |
||
| entity-unavailable: | ||
| status: exempt | ||
| comment: This integration cannot be unavailable since it's a local calculation. | ||
| integration-owner: done | ||
| log-when-unavailable: | ||
| status: exempt | ||
| comment: This integration cannot be unavailable since it's a local calculation. | ||
| parallel-updates: done | ||
| reauthentication-flow: | ||
| status: exempt | ||
| comment: This integration does not require reauthentication, since it is a local calculation. | ||
| test-coverage: | ||
| status: todo | ||
| comment: |- | ||
| The following points should be addressed: | ||
|
|
||
| * Don't use if-statements in tests (test_jewish_calendar_sensor, test_shabbat_times_sensor) | ||
|
|
||
| # Gold | ||
| devices: done | ||
| diagnostics: done | ||
| discovery-update-info: | ||
| status: exempt | ||
| comment: This is a local calculation and does not require discovery. | ||
| discovery: | ||
| status: exempt | ||
| comment: This is a local calculation and does not require discovery. | ||
| docs-data-update: todo | ||
| docs-examples: todo | ||
| docs-known-limitations: | ||
| status: exempt | ||
| comment: No known limitations. | ||
| docs-supported-devices: | ||
| status: exempt | ||
| comment: This integration does not support physical devices. | ||
| docs-supported-functions: done | ||
| docs-troubleshooting: | ||
| status: exempt | ||
| comment: There are no more detailed troubleshooting instructions available. | ||
| docs-use-cases: todo | ||
| dynamic-devices: | ||
| status: exempt | ||
| comment: This integration does not have physical devices. | ||
| entity-category: done | ||
| entity-device-class: done | ||
| entity-disabled-by-default: done | ||
| entity-translations: done | ||
| exception-translations: done | ||
| icon-translations: done | ||
| reconfiguration-flow: done | ||
| repair-issues: | ||
| status: exempt | ||
| comment: There are no issues that can be repaired. | ||
| stale-devices: | ||
| status: exempt | ||
| comment: This integration does not have physical devices. | ||
|
|
||
| # Platinum | ||
| async-dependency: todo | ||
| inject-websession: | ||
| status: exempt | ||
| comment: This integration does not require a web session. | ||
| strict-typing: done | ||

Uh oh!
There was an error while loading. Please reload this page.