architecture
architecture copied to clipboard
Official deprecation policy
This is an initial proposal and will need to be fleshed out further. I will update as commentary dictates
Context
In the code base today, deprecation of functionality (compared to outright removal of that functionality) is applied sporadically. Often, breaking changes are forced into the next full release without much consideration of the long-term impact; it is often the case that deprecation is only apparent when a peer review exposes the possibility.
Questions
First, we should define some overarching rules around deprecation:
- What is the standard length of time for deprecation before the change is locked in place (measured in minor versions)?
- Can the deprecation timeframe change? If so, what actions/inputs/etc. change it?
- What happens if subsequent PRs attempt to alter the same surface area before the deprecation period ends?
Second, whenever the breaking change label is applied to a PR, it should trigger some sort of "deprecation review." The purpose of this review is to determine whether the requested breaking change can sustain a deprecation period (or whether it needs to be locked in ASA).
Third, we encourage (require?) the author of the PR (and/or the codeowner of the affected surface area) to:
- create an issue related to removing the deprecation notice and locking in the change.
- assign a version milestone to that issue (consistent with the deprecation period) so that its resolution must coincide with the promised release timeframe.
Proposal Notes
Cases
- Deprecation of a single
configuration.yamlkey - Deprecation of multiple
configuration.yamlkeys - Complete removal of
configuration.yaml-based configuration - Deprecation of non-configurable feature
Consequences
Pros
- Fewer "immediate" breaking changes
- More breathing room for end users to accommodate forthcoming changes
- Consistent reminders (via logging) of still-in-use deprecated features
- A standardized approach that transcends PR author(s) or reviewer(s)
Cons
- More administrative overhead for both author(s) and reviewer(s)
- Code reviewers will need to remember to look for deprecate-able surface areas (perhaps a subsequent ADR should handle a code review checklist?)
There are different paths here depending on where the integrations starting point is:
In all cases where yaml is present, there is a possibility to mark it with cv.deprecation. This should be required.
Either integration already have config flow and imports yaml config -> dev can immediately deprecate yaml config giving users two releases to remove the yaml config
CONFIG_SCHEMA = vol.Schema(cv.deprecated(DOMAIN, invalidation_version="0.109"), {DOMAIN: cv.match_all})
Or integration doesn't have config flow and should add a config flow and trigger an import from the platform/integration it is configured from. Also here give the users a couple of releases to clean up yaml
@Kane610 Good stuff; I'll add it to my notes.
The one thing to keep in mind with your examples is that they focus on configuration.yaml or config entries. The question is: are those the only deprecate-able scenarios? Are there any other functional areas (besides integration configuration) where we might want to dictate standards around deprecation?
We won't be able to touch all scenarios – e.g., if I change the unit on a sensor, that's a breaking change that I can't deprecate beforehand – but it's worth some consideration.
Another example that is similar but slightly different: These days homekit_controller only uses config entries. All the pairing crypto kes are stored there as well. I recently deprecated and removed some legacy JSON files that were squirreled away in a .directory inside the config folder used to store these keys. They were only ever written to by HA itself, they were not user config. Most users likely didn't know the file was there. But if the user didn't upgrade to a release within a certain window (after config entry support was added and before support for reading the deprecated files was removed) they would have had to re-pair all their homekit accessories.
They had a year were HA silently migrated from the legacy format to config entries. Then we added a deprecation warning a few releases ago, and they should be gone in the next release. The deprecation warning in the logs was kind of pointless - if you had a new enough HA to see it then it didn't affect you.
Another: There has been a bit of talk recently above moving in the direction of having more entities per device and making better use of the device registry (yay!), rather than trying to squeeze all the data into a single entity. This is the kind of thing that might cause breaking changes. E.g. Deprecate AirQuality or Making battery charging yes/no state seperate to battery charge %.
Absolutely, I have on my to do for deCONZ to move a type of binary_sensor over to become a switch. That must be some sort of breaking change, probably a welcome one but a BC none the less. And it is hard to do anything but happen, there is no in-between here. It will just happen.
What about updating documentation in parallel with deprecations? I've just marked an integration's YAML config as deprecated for removal after two more releases while it continues to import YAML into persistent config entries. Should the docs describing how to create YAML configs be removed now?
~I just added the 0.114.0 milestone as a means of tracking my own commitments to remove deprecated functionality by that version. I personally like this idea (creating future milestones to track committed changes).~ We can't do that yet.
Two examples of different perspectives in the last few days:
- https://github.com/home-assistant/core/pull/34854 – in this case, the
configuration.yamlremoval isn't immediate; it's deprecated until 0.112.0. - https://github.com/home-assistant/core/pull/34825 (mine) – in this case, I didn't see anything to lose by removing the config outright (since it would already have been imported into a config entry).
I don't know the Fortigate use case well enough, but it seems that both of these are appropriate given their circumstances?
I also think that we should define what type of breaking change we want.
For example, if we remove 1 YAML config option but there are still others around, it gives a better experience to just warn about that key, but ignore it and still allow it to be valid config if it's passed in. That way the integration will still set up. We can then remove that key in a future release, or just never, and just warn about it whenever it's present.
This architecture issue is old, stale, and possibly obsolete. Things changed a lot over the years. Additionally, we have been moving to discussions for these architectural discussions.
For that reason, I'm going to close this issue.
../Frenck