Remove YAML config for synology_dsm#42777
Remove YAML config for synology_dsm#42777Quentame wants to merge 3 commits intohome-assistant:devfrom
Conversation
dc5afc1 to
ab62ab9
Compare
|
|
|
Hi @Quentame , |
|
Hi ! |
|
Added you to the repo. TODO:
While writing this, I am thinking that it would be easier to remove this config, and I think we should remove it.
What do you think @mib1185 ? |
|
Hi @Quentame , I have already pushed a commit, where I added an additional setup step to define, if user wants to limit/filter the disks and volumes to add, or not 😉
I would suggest to implement something like "reconfigure is needed" as I already have seen on another integration 🤔 |
| vol.Required( | ||
| CONF_DISKS, default=self.syno_info.get(CONF_DISKS, []) | ||
| ): cv.multi_select(self.syno_info.get(CONF_DISKS, [])), | ||
| vol.Required( | ||
| CONF_VOLUMES, default=self.syno_info.get(CONF_VOLUMES, []) | ||
| ): cv.multi_select(self.syno_info.get(CONF_VOLUMES, [])), |
There was a problem hiding this comment.
Devices should never be undefined
--> self.syno_info[CONF_...]
There was a problem hiding this comment.
In case of an virtual DSM running on virtual machine manager, there are no disks available (see #42523), but yes for volumes - they never should be undefined ... I will change it
|
Yep, good work 👏🏼, but is this (future) work worth it ? We should not code this for the fun of doing it but for the good of the integration. |
34de617 to
5bb9cf9
Compare
|
Yeah, I fully agree with you, that we should not code for the fun of doing it but for the good of the integration and we should focus on fixing issues or adding new features to the end user. 👍 |
5bb9cf9 to
9c3f426
Compare
9c3f426 to
506d537
Compare
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
|
This PR has been in a draft and running stale without activity since November. |
Breaking change
Synology DSM has fully transitioned to configuration via UI. YAML configuration is no longer supported after being automatically imported for several releases.
Proposed change
Following ADR-0010: https://github.com/home-assistant/architecture/blob/master/adr/0010-integration-configuration.md
In draft because
CONF_DISKSandCONF_VOLUMESneed some love to be configured.It was only possible to set them at import, so we should either:
What should I do ?
Type of change
Additional information
Answer to #42539 (comment)
Checklist
black --fast 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..coveragerc.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: