Problem/Motivation
We have a few BC layers that are hidden behind settings that have no UI. That means there is no awareness for users that they rely on that and they also have no way to change that setting without manual config import/export or drush.
Some have related deprecation messages with a @trigger_error(), but these are meant for developers & tests, they are not visible on production sites. As I wrote in #3009854: Fix "The "serializer.normalizer.file_entity.hal" normalizer service is deprecated: it is obsolete, it only remains available for backwards compatibility." deprecation error, where I'm actually removing one of those:
@trigger_error() has one purpose IMHO. To fail tests when someone has code that relies on a deprecated API/behavior. Its target audience are developers. I think it is very unlikely that tests are going to have that setting enabled.
This behavior is controlled by configuration, there is no alternative thingy that a developer should be using instead, not *inside* Drupal. They need to update their app or decoupled website, but a @trigger_error() wouldn't help them discover this problem.
We could easily add a @trigger_error() inside the if condition, but unless we start to actually log these to watchdog in 8.9, I don't that makes much sense.
IMHO, if we need something, then that's a hook_requirements() check that shows a warning or so that they might be relying on the deprecated thing that will go away in 9.x and should disable it.
Proposed resolution
Implement my suggestion, show a warning on the status page about enabled BC layers. Possibly provide a way to switch it?
I've created this for hal.module but there's at least one more setting in rest.module about the correct property types.
We also have a precedent for this now since #3039026: Deprecate file_directory_temp() and move to FileSystem service, which added it for old temporary path configurations if we can't just remove it.
Remaining tasks
For D9, we'll also need an update function that removes the old setting, whether or not it was enabled. That can be done in the issue that removes the BC layer, e.g. #3034062: Remove hal.module BC layers
Comments
Comment #2
berdirComment #3
xjmThis would be a minor-only change. Since 8.9.x and 9.0.x are now in beta, I'm moving this to 9.1.x. Thanks!
Comment #7
spokjeThe
halmodule has moved out of Drupal Core and into a Contrib Module.Moving this issue to the Contrib Module queue.
Comment #8
berdirThis was actually a task that was only relevant for D8, because those BC layers have been removed in D9. I'd suggest moving back and closing as outdated.
Comment #9
spokjeThanks @berdir.
Less = more, returned to sender and closed as outdated