Problem/Motivation

Often, in the course of improving code in Drupal, it becomes necessary to provide code paths that are used specifically to maintain our backwards compatibility promise. It's not appropriate to mark the code as deprecated, as it may include conditionals that are expected to be run, or it may include data transformations that update legacy data to match current expectations while remaining idempotent for modern data, and are thus run on every data set currently.

A common example is providing for dependencies that are added to constructors in a backwards compatible way. In this case, a new optional parameter is added to the end of the constructor, and then a BC layer is added to detect if the parameter is missing, and load it from the container if it is, usually triggering a deprecation error. There are numerous other examples in core. The migration system deprecated the use of CckFieldPlugins in favor of the replacement FieldPlugins, and requires a fair amount of code to manage still-existing CckFieldPlugins that is not technically @deprecated, but should be removed when the Drupal 9 branch is open. Views has a method View::fixTableNames() which exists to update views which may have been saved prior to the 8.3 revisionable entity updates, and imported. Eventually this method should be removed, perhaps in Drupal 9, perhaps later. This method is currently marked deprecated, but should not be, as it is correctly still in use.

Several other examples have been noticed while working on cleaning up deprecations in #3038170: Drupal core's own deprecation testing results

Currently we handle this in a few ways. Sometimes we mark methods as deprecated, which is not correct, and won't work as we are now enforcing proper use of the @deprecated tag. Sometimes there is a comment in the code pointing out that the code exists as a BC layer and should be removed. Sometimes we file follow-ups, and sometimes we do nothing to mark that the code needs to be removed in a later version.

The current documentation on this is at https://www.drupal.org/core/deprecation#how-codepath and says only:

For example when we have a code path handling a deprecated key in a YAML file.
Add @trigger_error('...', E_USER_DEPRECATED) at the start of the code block that handles the backwards compatibility layer. DO NOT add an @deprecated phpdoc annotation to the method or function docblock that contains the code path as this causes IDEs to mark the entire method as deprecated. For example:

// @todo

Other documentation on that page suggests where to trigger an error in certain situations, but does not document ways to mark a particular code block as deprecated and needing to be removed. The main purpose of this policy is to designate a way to mark these code blocks as being scheduled for removal in the code so that we can easily create and use automated tools to make sure that these paths are removed when we open a new major version branch.

The below proposal is a starting point for discussion on how to reign this code in, mark it properly, and make sure it is easy to remove in the next major version.

Proposed resolution

First and foremost, any code that is provided for a bc layer and expected to be removed should have a follow-up issue filed to remove it at the time it is committed. The code should contain an @todo describing what is to be removed/changed, and a link to the issue. Should we have a new tag for this so we can scan the code when the next major branch is open? @bc or something?

When possible, BC layer code should be surrounded by a conditional detecting if it is needed, and if so, an E_USER_DEPRECATED error should be triggered.

If the needed logic is more complicated than one or two lines, it should be moved into its own method (where appropriate). This method should be marked @internal, private, and final. @bc? In this case, both the method and the location from which it is called should be commented with the @todo tag, and with a link to the follow-up issue.

Ideally a patch to remove the code could be posted to the follow-up issue at the time it is created, but keeping the patch rolled correctly may prove too much.

Remaining tasks

Discuss if such a policy is even needed.
Determine best practices
Document

User interface changes

none

API changes

none

Data model changes

none

Release notes snippet

Comments

mikelutz created an issue. See original summary.

mikelutz’s picture

Issue summary: View changes
catch’s picture

Yeah a policy/guidelines for this makes sense and the proposed resolution looks like a good start. This code is more tricky technical debt than actual deprecated things but harder to find.

Would probably got for @backwards_compatibility or @bc_layer if we go for a new tag.

mikelutz’s picture

It did occur to me that a new tag and filling a follow-up might be redundant. If we do indicate each need removal or adjustment with a in code tag and documentation, we might be able to remove all bc code in one big patch. I'm not sure if that is a better way to tackle the removal or not. We would also be dealing with @legacy tests that would need to come out at the same time.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.