Skip to content

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.

Sign up for GitHub

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

Merged
joostlek merged 15 commits into from
Sep 10, 2025
Merged

Jewish Calendar quality scale #143763

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
089dc7b
Initial commit quality scale
tsvi May 7, 2025
b22cba4
Entity translation is done
tsvi May 9, 2025
cd1d778
Parallel updates and appropriate polling are done
tsvi May 15, 2025
752c4e7
We don't need any additional action exceptions
tsvi May 19, 2025
bd7fc9c
Diagnostics is done
tsvi May 19, 2025
78145bc
Icon translations are done
tsvi May 20, 2025
953d81a
Exception translations are done
tsvi May 21, 2025
c562035
Merge branch 'dev' into jcal-qs
tsvi Jul 9, 2025
be8ed1f
We do subscribe to events
tsvi Jul 13, 2025
84165e5
Add comment regarding unique config entry
tsvi Jul 13, 2025
22648e0
Add comment regarding if statements in tests
tsvi Jul 13, 2025
fbe7d78
Fix comments
tsvi Jul 13, 2025
0017344
We have a service device
tsvi Jul 13, 2025
5a9e44c
Merge branch 'dev' into jcal-qs
tsvi Jul 14, 2025
b1a92c7
Merge branch 'dev' into jcal-qs
tsvi Aug 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions homeassistant/components/jewish_calendar/quality_scale.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
rules:
Copy link
Member

@joostlek joostlek Jul 9, 2025

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

@tsvi tsvi Jul 13, 2025

Choose a reason for hiding this comment

The 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.
Sometimes it makes sense knowing the day/month/year value by itself, but in most cases, the date is good enough.
It's a real attribute of the sensor.

Copy link
Member

@joostlek joostlek Jul 13, 2025

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

@tsvi tsvi Jul 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had this discussion here: #116252 (comment)

# Bronze
action-setup: done
appropriate-polling: done
brands: done
common-modules: done
config-flow-test-coverage: done
Copy link
Member

@joostlek joostlek Jul 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we test that the option flow refreshes the entry?

Copy link
Contributor Author

@tsvi tsvi Jul 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Refreshes" meaning that the updated values are taken into consideration?

Copy link
Member

@joostlek joostlek Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes!

Copy link
Contributor Author

@tsvi tsvi Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that included in this test?
https://github.com/tsvi/home-assistant/blob/b1a92c79384a3985ba6431522a945fda0271c3fd/tests/components/jewish_calendar/test_config_flow.py#L96

config-flow: done
dependency-transparency: done
docs-actions: done
docs-high-level-description: done
Copy link
Member

@joostlek joostlek Jul 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can remove the
image
parenthesis here

Copy link
Contributor Author

@tsvi tsvi Jul 14, 2025

Choose a reason for hiding this comment

The 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:
status: exempt
comment: Entities of this integration do not explicitly subscribe to events.
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: done

# Silver
action-exceptions: done
config-entry-unloading: done
docs-configuration-parameters: done
docs-installation-parameters: done
Comment on lines +36 to +37
Copy link
Member

@joostlek joostlek Jul 9, 2025

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

@tsvi tsvi Jul 14, 2025

Choose a reason for hiding this comment

The 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: done
Copy link
Member

@joostlek joostlek Jul 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also expect tests for the runtime of the integration

Copy link
Contributor Author

@tsvi tsvi Jul 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually we have those, but maybe we should rename and reorganize those a bit:


# Gold
devices:
status: exempt
comment: This integration does not represent physical devices.
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 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 devices.

# Platinum
async-dependency: todo
inject-websession:
status: exempt
comment: This integration does not require a web session.
strict-typing: done
1 change: 0 additions & 1 deletion script/hassfest/quality_scale.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,6 @@ class Rule:
"itunes",
"izone",
"jellyfin",
"jewish_calendar",
"joaoapps_join",
"juicenet",
"justnimbus",
Expand Down