Qualify Music Assistant to Bronze Quality Level #143822
Conversation
|
Hey there @music-assistant, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
| entity-event-setup: done | ||
| entity-unique-id: done | ||
| has-entity-name: done | ||
| runtime-data: done |
There was a problem hiding this comment.
async_unload_entry should use MusicAssistantConfigEntry, as should async_remove_config_entry_device. This way we can remove the type overwrite
mass_entry_data: MusicAssistantEntryData = entry.runtime_data| brands: done | ||
| common-modules: done | ||
| config-flow-test-coverage: done | ||
| config-flow: done |
There was a problem hiding this comment.
Only have things in the try block that can raise
try:
self.server_info = await get_server_info(
self.hass, user_input[CONF_URL]
)
await self.async_set_unique_id(
self.server_info.server_id, raise_on_progress=False
)
self._abort_if_unique_id_configured(
updates={CONF_URL: self.server_info.base_url},
reload_on_update=True,
)| @@ -0,0 +1,70 @@ | |||
| rules: | |||
There was a problem hiding this comment.
self._attr_repeat = queue.repeat_mode.valueThis should become a map between HA options and MA ones, since MA can have UNKNOWN
| brands: done | ||
| common-modules: done | ||
| config-flow-test-coverage: done | ||
| config-flow: done |
There was a problem hiding this comment.
The strings are incorrect, there are no strings for the user step
There was a problem hiding this comment.
The strings can use more references to each other
| reauthentication-flow: | ||
| status: exempt | ||
| comment: Devices don't require authentication | ||
| test-coverage: todo |
There was a problem hiding this comment.
def load_and_parse_fixture(fixture: str) -> dict[str, Any]:
"""Load and parse a fixture."""
data = load_json_object_fixture(f"music_assistant/{fixture}.json")
return data[fixture]why is fixture the root object? Like, why do we add an extra layer in there
| comment: Integration is local push | ||
| brands: done | ||
| common-modules: done | ||
| config-flow-test-coverage: done |
There was a problem hiding this comment.
mock_get_server_info should not patch internals, but rather the library
| comment: Integration is local push | ||
| brands: done | ||
| common-modules: done | ||
| config-flow-test-coverage: done |
There was a problem hiding this comment.
It's not required to have await hass.async_block_till_done() after doing a config flow step as those generally do not fire events
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Breaking change
Proposed change
Make required changes and provide justification to promote the Music Assistant integration to the Bronze Level.
action-setup- Service actions are registered in async_setupcore/homeassistant/components/music_assistant/__init__.py
Line 52 in 78338f1
appropriate-polling- If it's a polling integration, set an appropriate polling intervalstatus: exempt
comment: Integration is local push.
brands- Has branding assets available for the integrationhttps://github.com/home-assistant/brands/tree/master/core_integrations/music_assistant
common-modules- Place common patterns in common moduleshttps://github.com/home-assistant/core/tree/dev/homeassistant/components/music_assistant
config-flow-test-coverage- Full test coverage for the config flowhttps://github.com/home-assistant/core/blob/78338f161f8dbfa2bde3bd10fed76f5539fdc8ad/tests/components/music_assistant/test_config_flow.py
config-flow- Integration needs to be able to be set up via the UIhttps://github.com/home-assistant/core/blob/dev/homeassistant/components/music_assistant/config_flow.py
data_descriptionto give context to fieldsSee updated
strings.jsonas part of this PRConfigEntry.dataandConfigEntry.optionscorrectlycore/homeassistant/components/music_assistant/config_flow.py
Line 129 in 6a01249
dependency-transparency- Dependency transparencyhttps://pypi.org/project/music-assistant-client/
https://github.com/music-assistant/client
docs-actions- The documentation describes the provided service actions that can be usedhttps://www.home-assistant.io/integrations/music_assistant/#actions
docs-high-level-description- The documentation includes a high-level description of the integration brand, product, or servicehttps://www.home-assistant.io/integrations/music_assistant/
docs-installation-instructions- The documentation provides step-by-step installation instructions for the integration, including, if needed, prerequisiteshttps://www.home-assistant.io/integrations/music_assistant/#configuration
docs-removal-instructions- The documentation provides removal instructionshttps://www.home-assistant.io/integrations/music_assistant/#removing-the-integration
entity-event-setup- Entities event setupcore/homeassistant/components/music_assistant/__init__.py
Line 137 in 8200c23
[X ]
entity-unique-id- Entities have a unique IDcore/homeassistant/components/music_assistant/entity.py
Line 62 in 6a01249
has-entity-name- Entities use has_entity_name = Truecore/homeassistant/components/music_assistant/entity.py
Line 23 in 78338f1
runtime-data- Use ConfigEntry.runtime_data to store runtime datacore/homeassistant/components/music_assistant/media_player.py
Line 145 in 6a01249
test-before-configure- Test a connection in the config flowcore/homeassistant/components/music_assistant/config_flow.py
Line 74 in 6a01249
test-before-setup- Check during integration initialization if we are able to set it up correctlycore/homeassistant/components/music_assistant/__init__.py
Line 66 in 78338f1
unique-config-entry- Don't allow the same device or service to be able to be set up twicecore/homeassistant/components/music_assistant/config_flow.py
Line 67 in 78338f1
Type of change
Additional information
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: