[policy, no patch] Make removal of deprecated usages a feature release blocker

Making it a feature release blocker will presumably only eliminate deprecated usages from files that are touched by patches. How about a clean-up plan to get rid off the backlog of deprecated usages, worth doing in it's own right? A lot would be suitable for novices and/or code sprints, and it could be broken down by module/component, or done per-deprecated-function.

@Pasqualle you are completely correct that core is using way too much deprecated code and things have been deprecated in ways that make providing BC not that simple. However we have only supported deprecation testing via @trigger_error('A message', E_USER_DEPRECATED) since 8.4.0 and we have been working on not skipping any of these messages - see #2959269: [meta] Core should not trigger deprecated code except in tests and during updates and there has been work on adding @trigger_error() calls to deprecated code paths which were added before the policy. So the good news is that core is not adding anymore deprecations without removing all usages first and we are chipping away at the technical debt. Yes it is a lot of work and all help is appreciated. But I disagree that there has been no improvement here after 5 months. What the raw number analysis misses is that we no longer skipping any deprecations from Twig allowing us to upgrade to Twig 2.0 and we're no longer skipping a couple of Symfony deprecations making the route to Symfony 4 easier. Plus I think if you analyse the numbers you'll find the large majority of the deprecated code usage picked up by PHPStorm is from Tests and usages of things like \Drupal\KernelTests\AssertLegacyTrait::assert* - just fixing all the usages of the deprecated methods in AssertLegacyTrait would have a massive impact on your number but I would argue that doing that find&replace is not a great deal of technical debt - unlike trying to fix #2961861: Remove usage of WriteCheckSessionHandler - for example.

We need to:

• Add @trigger_error() to deprecated code paths in core ASAP and remove usages
• Remove ::getSkippedDeprecations()
• Surface deprecated code usage in contrib tests

And then promise of easier updates to Drupal 9 can be realised.

A couple of other points are

• the removal of all deprecated code usage is already a suggested pre-requisite of Drupal 9 - see #2608062: [META] Requirements for tagging Drupal 9.0.0-alpha1
• there is also nothing to stop us re-deprecating something like AssertLegacyTrait to be removed in Drupal 9 if we feel that concentrating on removing something like that in Drupal 8 is too disruptive.

Yes, new deprecations are coupled to removing usages.

We have a long backlog of deprecations added prior to the release of 8.0.x (and to a lesser extented 8.0.x-8.4.x) which are still being worked through. If you look at issues like #2848161: [meta] Replace calls to deprecated db_*() wrappers there's a tonne of work going on to clear that backlog.

Around the time we're ready to open 9.x, we'll need a patch that removes all the deprecated functions/methods/classes and this will fail unless all the usages have been removed by that point. Ideally that's going to be around the same time as the penultimate 8.x minor release so 6+ months before the 9.x release itself. Any usages that haven't been removed from core by that point we'll have to revisit whether to extend the deprecation or not, but there's another year or so before we get there, so time still to clean these all up.

I think one thing that could help here is a meta issue that tracks progress and remaining work. Looks to me like there's also some confusion about when to use the deprecated tag and when to use the @deprecated tag. The former looks like it has too few issues. The latter looks like it clutters the usage removals of what's already deprecated with proposals for new deprecations.

Most constructor parameters are not part of the API, so are fair game to change. #2624770: Use more specific entity.manager services in core.services.yml is an example of an issue that's doing so.

For sorting out the issue tags, I opened #3001275: Decide on and publish the meaning of the "deprecated" and "@deprecated" issue tags..

There's a meta-issue here #2959269: [meta] Core should not trigger deprecated code except in tests and during updates, but it would need all the active issues added as child issues to work properly as a tracking issue.

> Most constructor parameters are not part of the API, so are fair game to change. #2624770: Use more specific entity.manager services in core.services.yml is an example of an issue that's doing so.

Far from fair game IMHO. In most issues, we're doing everything we can do avoid breaking contrib with additional constructor arguments, by adding them as optional arguments with a fallback. Specifically the issue you mentioned goes to great length to keep the disruption at an absolute minimum, it even adds a trait that adds a fallback for deprecated/removed properties so that subclasses that are still using those don't break, including a @trigger error and full BC for subclasses calling that constructor.

As recently commented in #2850037: Replace all calls to db_like(), which is deprecated, as a maintainer of a lot of contrib, I can say that those kind of constructor changes are one of the most frequent reasons for broken contrib modules due to core changes in minor versions. And I would suggest to document that kind of best-practice BC rule on new constructor arguments as the standard.

And yes, the issue in #13 is the one we (mostly @alexpott) have been working with. I do agree it would be good to have more support/focus on that kind of technical dept issues (as well as bugs and so on) and not just the official and not-so-official initiatives :) For example that entity.manager issue has IMHO been ready since june and entity.manager is one of the larger and trickier deprecations and it would really help if we could clean that up and add proper @trigger_errors() to it.

Beside that, I'm not sure how this issue exactly helps to get rid of those deprecated usages or what it's purpose is. The suggestion is makes is clearly flawed, with time-boxed, regular minor releases, pretty much *nothing* can block that. That's kind of the point.

> Is there any viable solution for that, or the plan is to clean those only in D9 branch?

That's the thing. 9.x can *not* make any API change at all as per the defined rule, contrib modules must be able to work with both 8.latest and 9.0. That means we can only remove deprecated usages in a backwards compatible way and I'm sure there are dozens of really hard challenges waiting on us there.

it would be good to have more support/focus on that kind of technical dept issues

Then maybe better to file more novice issues to "replace deprecated usage"?