[Policy, no patch] Clearly denote when @deprecated code is slated to be removed

+1 so hard.

Doesn't really matter if we make it up, I dont think it's a very common thing to do.

While I didn't agree with the original title, the new one is completely OK. We do not replace @deprecated always but most of the time -- yes. For example, I remember deprecating #after_build in ... um... Drupal 5? 6? something and it's still there.

Since when @deprecated doesn't mean "remove before 8.0"?

Many, if not most, of our @deprecated calls say @deprecated as of Drupal 8.0.
That implies that they will be around *after* 8.0

It doesn't make much sense to mark methods as @deprecated as of Drupal 8.0, which is not yet released. Either those methods are deprecated now, and then we need to remove them before Drupal 8.0, or they should not be marked as deprecated at all.

Think of it as @discouraged then.

Suppose it would be great to make meta issue with list of all stuff that we are going to remove. Sometimes it's really discouraging that we have issues like #2157695: Remove unused _comment_get_modes()

If you want to make it wholesale then it's easier to document what we mean by @deprecated. Or you want to do one wholesale now and then before release go back and rename the rest :) ?

As well as providing a todo list for core development, this change is intended to prevent modules being upgrade to support Drupal 8 alphas/beta from using functions that won't be available in 8.0. The concern was that if functions were just marked as @deprecated then people wouldn't know which ones could still be used and which must be removed.

On irc webchick mentioned that renaming functions/classes had been considered as an option (e.g. variable_get becomes __donotuse__variable_get). She said the argument against this was that it would require contrib module developers to do two conversions: first to the new name, then to the ultimate replacement.

I don't think this should actually be the case though. I would expect all functionality that we are intending to remove in Drupal 8 to already have a replacement. If it doesn't, we should be asking if we're really going to be able to remove it in 8.0 anyway.

I don't think we can do one 200k patch, because we have different kinds of @deprecated functions in core at the moment:

1. Stuff that is marked @deprecated, and is also not used any more in core because the rest of core is updated to use the new stuff, and isn't used much in contrib.

2. #1 except that it's commonly used in contrib still, so removing will require any mid-port contrib modules to update code.

3. Stuff that is marked @deprecated, where the rest of core isn't updated yet but could be done quite trivially, and where current core patches might use the deprecated functions too.

5. Stuff that is marked @deprecated, where the new stuff doesn't exist yet or requires major architectural work/conversions still to go in (lots of the menu system right now).

6. Stuff that is not marked @deprecated at all, but that we actually want to remove before release anyway, which is a subset of 2-4. (variable_init/set/get/del() are in this category).

I'd suggest that anything in category #1, we should try to do these asap. However we should keep the removal patches isolated - anything that needs existing code ported (i.e. category #2 in that list) should have one issue for that, and one issue for the removal. This is so unintended head breakage can be isolated to the removal commits rather than the conversions as much as possible.

The other types need more discussion.

Also worth remembering that we'll very likely want to mark certain things @deprecated even once we get to RC or post release - because there's going to continue to be conversions of certain procedural functions to classes, or things we just missed etc. So we're going to have @deprecated stuff in 8.x.x somewhere even if we clean out all the old crap before RC.

Alex just corrected my example of variable_get() being not deprecated, we actually did that in August and I committed the patch. However it was not deprecated for at least a year after CMI got in, so would have been a good example four months ago!

1) I agree we should just remove these
2) I think we should remove these too, flagged up with change notices and a blog post on drupal.org/planet. We should do the removal immediately after an alpha release, so module authors can work on their conversions using that alpha release.
3) Hopefully this list is small and can be processed quickly. They sound like issues which could be good for novices.
4) This point has already been removed :P
5) Eek!
6) If there are any more examples that we know about, they should be promptly marked deprecated so people don't write new code using them. The ones we don't know about will have to be evaluated on a case-by-case basis for effort required to remove vs effort required to maintain a backwards compatibility layer.

I think 4) was meant to be "stuff that has a replacement but is still used in core". This is the category that I'm advocating renaming.

OK I agree with trying to update the existing @deprecated to whatever we pick in one big patch, especially if we also go ahead and remove obvious ones (field_read/get_*() are the ones currently on my mind) independently of that issue. The actual removal patches are trivial to roll and roll back, even compared to deciding what's removed vs. left in.

If there's something controversial, that can always be split out of the one big patch if necessary.

It would be a good idea to stop making intermediate changes and focus on breaking as many APIs as quickly as possible.

Our limited time is better spent with actually removing all @deprecated functions and converting all the remaining calling code.

The list of things that need to be removed, converted, and cleaned up is extraordinarily large. It affects procedural code, but also and worse, even new object-oriented code. Most removals/clean-ups will require API changes.

Out of all @deprecated things, only the \Drupal class should remain for 8.0, whereas even that is highly debatable.

[→ the only valid use-case are procedural hooks and theme preprocess functions, which both could simply get a $container injected as a mandatory first argument, hence eliminating the need for \Drupal — if that won't happen, then the non-existing @deprecated tag but intended meaning of the \Drupal phpDoc needs to be corrected accordingly.]

All of the existing @deprecated functions are only causing confusion for developers who are diving into D8, since call chains are needlessly interrupted by aliens.

The longer @deprecated functions exist, the slower the adoption. To speed up adoption, remove all legacy stuff as soon as possible.

Instead of debating how to tag obsolete code, it would be a much better idea to found a dedicated, formal effort (meta/issue-tag) in order to establish a 360° view on everything @deprecated (tagged or not), so as to track and resolve all the major challenges involved.

There are probably only a handful or dozen of people who are able to overlook the grand picture and sad mess that exists right now. The earlier we have an orchestrated effort, the sooner people are able to work on it, including novice developers.

You may call it coincidence, or you may call it a testament of a thriving community that isn't afraid of change, but by adopting HttpKernel and DependencyInjection, and by actually going out of our way and rewriting all code, we collectively signed up to release Drupal 8.0 as a 98% object-oriented application. Aside from a few exceptions, it would be an epic mistake to release a half-baked state in between. Drupal 8 will only be successful if we can deliver a sane and solid architecture and developer experience.

You may call it coincidence, or you may call it a testament of a thriving community that isn't afraid of change, but by adopting HttpKernel and DependencyInjection, and by actually going out of our way and rewriting all code, we collectively signed up to release Drupal 8.0 as a 98% object-oriented application. Aside from a few exceptions, it would be an epic mistake to release a half-baked state in between. Drupal 8 will only be successful if we can deliver a sane and solid architecture and developer experience.

This is a really nice summary of the concerns that I voiced this summer, and that lead to the "it's freezing but not frozen" approach.

If we release an hybrid monstrosity, we should not be surprised that people are running away screaming.

@sun: It would be a good idea to stop making intermediate changes and focus on breaking as many APIs as quickly as possible.

Two thoughts on this:

1) I love it, go ahead.

2) Other people will hate it, be cautious.

Helpful, I know.

But how about a case study: #2165259: Create node_type_example This is a D7 API that was severely broken by D8, and we like it.

As to the essential question: @deprecated is supposed to be a tag that warns consumers of the API that the API is about to change. This is what I've been assuming it means, since that's what it means. :-) But apparently I was wrong.

Note one important fact: Consumers of the API are people who are developing against the API, not people making the API. That means the people in this comment thread are not the audience for the @deprecated tag, they are the ones who should be having a design discussion, not a tag-semantics discussion.

That's one thing that's frustrating about Drupal for me: No one ever has a design discussion or determines requirements. It's all 'how do we use @deprecated this week?'

Consumers of the API are people who are developing against the API, not people making the API.

That means the people in this comment thread are not the audience for the @deprecated tag, they are the ones who should be having a design discussion, not a tag-semantics discussion.

The @deprecated tag never intended to have an audience in the first place. We introduced it in order to make progress. We made very good progress, and we actively continue to do so.

The fact that we adopted a phpDoc standard for documenting deprecated functionality (in order to make progress) has no meaning and should not have any kind of impact on product release and architecture politics.

What matters is that we deliberately chose to evolve in a progressive way. With the sole intention to keep patches small and to avoid to break 120 other patches in the queue. That is a development/workflow decision, not a product/release decision.

The goal of this issue is to mark obsolete code as obsolete, while retaining a sane meaning of a newly introduced @deprecated marker, in a product that is not released yet. That is a product/release decision.

Drupal core (1) never shipped with obsolete code and (2) never shipped with deprecated functions.

Given recent Drupal core release management discussions, (2) may be acceptable for 8.1.0, but 8.0.0 is a completely different kettle of fish.

It does not help anyone to carry around legacy code for a new major product release. It is the exact opposite: (1) DX disaster. (2) Confusion. (3) Fundamental architectural problems. (4) Lack of separation of concerns. (5) Massive context/global-scope problems. (6) Undefined APIs. (7) The system that claims to make you productive causes you to give up, resign, and forget about it.

For Drupal 8.0.0 - a complete rewrite of Drupal core - there's only one sane goal to be met:

Everything @deprecated and @obsolete is gone.

We collectively signed up for it. We have to bite the bullet, whether we like it or not. It's the only sane path forward; it's the only way to call out success after an epic effort of rewriting an entire application for a whole new architecture.

We're almost there. Stopping now is like giving up at 90%.

Let's stop to discuss and manage the past. Let's focus on the future.

Drupal core (1) never shipped with obsolete code

It's never knowingly done this, but we've shipped with plenty of dead code, some of which has been around for years before it was removed.

and (2) never shipped with deprecated functions.

We've also done this, they were just never identified as such in the code base. For example node_load() was a one line wrapper in Drupal 7 too.

Given recent Drupal core release management discussions, (2) may be acceptable for 8.1.0, but 8.0.0 is a completely different kettle of fish.

It's quite feasible that we'll mark a function deprecated between RC1 and RC2, a current example might be node_load() which is not marked @deprecated at all. If we did that, then we'd definitely ship 8.0.0 with some functions marked @deprecated and that would be fine.

The situation right now is a complete mess in terms of functions marked deprecated that can't be removed yet, ones that could just be ripped out, and ones that should be ripped out but aren't even marked yet.

But arbitrarily saying that 8.0.0 can't possibly ship with any deprecated functions at all doesn't really fit in with that IMO - unless you think it's impossible we'll find more deprecated stuff that's not been identified yet, which would be a first for any of the past 4-5 major releases at least.

we've shipped with plenty of dead code

I assume you're referring to truly dead code that simply existed while no one noticed, because no one was even aware that the code existed. That is absolutely true, but that is a completely different story.

That sense of "dead code" is completely different to intentionally shipping with "code that is supposed to be dead", in the sense of

function foo() {
  return bar();
}

That's a small but important difference.

We've also [shipped with deprecated functions], they were just never identified as such in the code base. For example node_load() was a one line wrapper in Drupal 7 too.

There was an issue to remove node_load() & Co for D7 already. The only reason for why that did not happen is that D7's "Entity API" was limited to querying, loading, and caching entities. The removal of node_load() would have introduced a major inconsistency with node_save() and node_delete(), which were not covered by that API.

It's about consistency. Consistency of legacy APIs. An argument that does not apply here. Or much rather, it applies in the exact opposite sense: These remnants are a thing of the past. All they achieve is inconsistency. Their mere existence encourages and endorses bad PHP4-esque habits in a system that claims to be modern otherwise.

Let's take a step back and let me put it differently:

1. Drupal has been clean and sexy in the past, attracting new developers and contributors, by re-inventing itself for each new major release.
2. This discussion is driven by an artificial/hypothetical pressure to get to a release ASAP, for no apparent reason.
3. To fulfill (2) but disregarding (1), we want to explicitly declare, retain, and manifest deprecated and obsolete functionality for a new major release of Drupal core.
4. The intention is to allow legacy developers to adopt D8 APIs quicker, by retaining old and obsolete code for longer, perhaps even for the entire major release cycle of 8.x.x.
5. The consequence is that new developers will have a very hard time of making sense of Drupal 8.
6. At the same time - tagged as @deprecated or not - all developers of the new system, legacy or not, will infinitely ask themselves why stuff like node_load() still exists at all, and worse, how and why any kind of code could reasonably use and depend on such a procedural function in an otherwise object-oriented and dependency-injected system in the first place.

In short: Everything that falls into the bucket of

We already know that this is @deprecated and should not exist in Drupal 8.0.0

…should be removed immediately. "Later" has non-trivial consequences on the product's entire architecture and APIs. "Later" means "Never" for Drupal 8.x.x, which will cause major headaches for both existing and new developers in trying to achieve their concrete goals.

We're designing, developing, and releasing a new product. Products exist to solve problems and use-cases. I guess I fail to see the use-case of shipping with deprecated/obsolete code?

But arbitrarily saying that 8.0.0 can't possibly ship with any deprecated functions at all doesn't really fit in with that IMO - unless you think it's impossible we'll find more deprecated stuff that's not been identified yet

I am arbitrarily saying that 8.0.0 can't possibly ship with any deprecated functions at all. — I agree the paradigm is new for Drupal, but we are in the best position to make it happen. We have proper interfaces, we have proper implementations, and we are injecting all dependencies. The time of quickly inventing a new node_load() is a thing of the past.

In fact, we are so well prepared that the only limiting factor in this new world order are exactly the procedural, deprecated/obsolete global-scope functions. They declare a parallel "API" that does not exist otherwise, and the entire system has to be bent and stretched to make those unnecessary legacy APIs available in the first place.

Instead of debating how to mark @obsolete things as obsolete things of the past, we should remove all of that stuff altogether as quickly as possible - and perhaps - we should start to think about the much more challenging product/release architectural design question of how to replace EntityInterface of 8.0.0 with EntityInterfaceV2 in 8.4.0?

Why are we stuck and try to bend our thoughts to the past, if the foreseeable future has much more interesting and epic challenges ahead of us?

Drupal 8.0.0 is going to be a major milestone in Drupal's history. It's a complete, utter rewrite of the entire application. We're looking at an extent that absolutely no one remotely had in mind when we started to work on D8 three years ago. — Not even the folks who always were strongly in favor of OOP would have ever imagined that we'd get this far within a single release cycle. — But somehow, magic, it happened. It's a total effin' rewrite of everything, but it happened!

Let's not undercut the amount of required clean-ups that are necessary prior to a stable release. Let's stop to compare this cycle with any other of the past, because it simply is not. Let's stop precluding that we'd have to retain any legacy code for anyone at all, because you'll hardly find someone that will say that "this makes sense" and solves anyone's problems.

Let's accept the fact that we've rewritten Drupal into a whole new thing and a completely new product. The only pressure that exists is to remove all deprecated/obsolete code that previously existed but which has absolutely no place in the new world order anymore. Let's make D8 sane and sensible. Let's make it shine.

This discussion is driven by an artificial/hypothetical pressure to get to a release ASAP, for no apparent reason.

No it's not driven by that at all. Once we get to RC, I don't plan to commit any API changes unless there's an emergency, but we can commit refactoring that marks existing procedural functions deprecated. Is it better to have the @deprecated function in core, or not to commit those patches because they 'break APIs'? At some point the API has to freeze, but that's potentially a while before the actual release - especially since we should have a proper RC schedule this time instead of releasing hours after critical bug fixes go in.

You've also completely ignored the fact that we have functions marked @deprecated in Drupal 8.x that we cannot remove because they are still in use and the conversions are not ready to commit - it's useful to be able to say those shouldn't be touched with a barge pole. I don't understand why you're objecting to clearly marking things for removal that will get removed, and conflating this with not removing anything.

Not to mention there is tonnes of legacy code in the application that are not one-line wrappers, but long and fairly complex functions - i.e. most of the bootstrap phases - those also need to be removed - again being able to tag those with something accurate would be useful.

The only reason that some @deprecated functions haven't been removed yet, is because I've asked for the patches doing the refactoring, vs. the ones actually doing the removal to be split up - this is because it's easy to re-roll deletions and there's been a lot of cross-commits that broke HEAD when removing things (again, because things aren't clearly marked as 'don't use this'). It's also nice to contrib developers to mark stuff as @something before actually nuking it - especially if it's something as widely used as node_load() - which again there is no live issue for at the moment.

Using @deprecated for "you should not use it, you have a good alternative" after RC1 is just good manners.

But we are not there yet. We are still in the "freezing" phase. Anything we do before RC1 should aim at completely nuking duplicated APIs, and making sure that Drupal 8 offers a consistent development experience.

Drupal 8 is not going to be a revolution in the CMS world (it has precisely zero new feature), so we absolutely need it to be a pleasant platform to develop on.

But we are not there yet. We are still in the "freezing" phase. Anything we do before RC1 should aim at completely nuking duplicated APIs, and making sure that Drupal 8 offers a consistent development experience.

So in that case, what's wrong with introducing a tag that says "we're going to nuke this", as opposed to "you might want to consider not using this at some point in the far future" so that people can distinguish between the two? Then when we start using @deprecated properly, it'll be a bit more predictable.

So in that case, what's wrong with introducing a tag that says "we're going to nuke this", as opposed to "you might want to consider not using this at some point in the far future" so that people can distinguish between the two?

That's fine, as long as we replace all the current @deprecated with @donotusewillbenuked.

Have you individually reviewed every usage? I'm pretty sure we have some premature tagging, along the lines of #788900: Deprecate and remove usages of arg() (although that's not actually tagged, but if it was there'd still not be a viable approach to make it actually obsolete).

If we changed @deprecatedbynothing to a normal @todo in those cases I'd agree 100% though.

To try and reign in the scope of this issue a bit, let's assume that we expect all alphas, all betas and Drupal 8.1 will ship with deprecated functions. (I've ignored Drupal 8.0 to avoid arguments about whether all deprecated functions should be removed before 8.0). This means we can leave the discussion about how and when to actually remove the APIs for later.

As webchick says, we've already got lots of deprecated code in vendor directories, so how about we just use one of their conventions? Specifically Symfony's:

From time to time, some classes and/or methods are deprecated in the framework; that happens when a feature implementation cannot be changed because of backward compatibility issues, but we still want to propose a "better" alternative. In that case, the old implementation can simply be deprecated.

A feature is marked as deprecated by adding a @deprecated phpdoc to relevant classes, methods, properties, ...:

/**
 * @deprecated Deprecated since version 2.X, to be removed in 2.Y. Use XXX instead.
 */

The deprecation message should indicate the version when the class/method was deprecated, the version when it will be removed, and whenever possible, how the feature was replaced.

from http://symfony.com/doc/current/contributing/code/conventions.html

In our case, the deprecation part of the message might read

@deprecated Deprecated since Drupal 8.x-dev, to be removed in Drupal 8.0

or for a beta-blocker:

@deprecated Deprecated since Drupal 7.47, to be removed in Drupal 8.0-beta1

Note I've used the word Drupal rather than version, to avoid confusion with vendor-provided APIs.

I can't see any reason not to implement this policy now, even if we take a bit longer to decide how and when we want to actually remove the functions. All functions should document what they should be replaced with, or link to the issue where their replacement is being developed.

The symfony standard continues:

A PHP E_USER_DEPRECATED error must also be triggered to help people with the migration starting one or two minor versions before the version where the feature will be removed (depending on the criticality of the removal):

trigger_error(
    'XXX() is deprecated since Drupal 8.x-dev and will be removed in 8.0. Use XXX instead.',
    E_USER_DEPRECATED
);

I support adding this too, but would advocate adding the docblock change now if using E_USER_DEPRECATED proves controversial.

I also wouldn't underestimate the task of identifing all the deprecated functions, including those not currently tagged, and deciding if we definitely are going to remove them for 8.0 or not.

In that case the Drupal class would be

@deprecated Deprecated since Drupal 8.0, to be removed in Drupal 9.0. See https://drupal.org/documentation/updating-away-from-drupal-backwards-compatibility-wrapper for help on what to use instead.

Not sure about the trigger_error() stuff; that feels like unnecessary hand-holding.

What you call hand-holding, I call providing a robust, predictable API. When I try my contrib module on a new beta I'll told what I need to update for it to work on future betas and how I can do that, rather than just a sudden, unexpected 'function not found'. If a function has been triggering E_USER_DEPRECATED for a beta or two, then a contrib module can't really complain when it gets removed in another beta.

My concern would be that we trigger too many of these errors, so perhaps a rule of thumb would be that when a function is no longer used by core it should start triggering E_USER_DEPRECATED, and at least one alpha/beta should be released with the function triggering E_USER_DEPRECATED before the function is totally removed.

My concern would be that we trigger too many of these errors, so perhaps a rule of thumb would be that when a function is no longer used by core it should start triggering E_USER_DEPRECATED, and at least one alpha/beta should be released with the function triggering E_USER_DEPRECATED before the function is totally removed.

+1

#31 and #34 gets the idea. Make it visible.

I just added an error_log() to all deprecated functions in bootstrap.ini and then loaded an anonymous front page.

Still a lot of variable_get() out there....

I just took the time to tag all issues that are cleaning up and removing deprecated stuff. We finally have a clear overview.

1. @deprecated is https://api.drupal.org/api/drupal/deprecated/8
2. and issues tagged with @deprecated will be removed soon.
3. See https://drupal.org/coding-standards/docs#deprecated for @deprecated coding standards.

#1876842: [policy, no patch] Use @deprecated to indicate a deprecated method or function had some example recommendation for including detail about which version of drupal might remove the depreciated thing.

It still leaves things like the Drupal class in a nebulous state, since I don't see any way we can avoid shipping with it in 8.0 really, and what it's deprecated in favour of is a massive refactoring of whatever code to be completely OO and dependency injected, not a simple "swap X function for Y method."

I think that is @todo issue nid plus a comment about using dependency injection properly, and should never have been marked @deprecated in the first place.

OK, so the coding standards already say basically the same thing as I proposed in #31 (ignoring E_USER_DEPRECATED), but we're not following them.

I've opened #2176961: Document deprecated code that will be removed before beta 1 to fix the documentation on the functions which are likely to be removed first, and will happily create more issues/patches if that one gets a positive response.

What I tried to say in all of my comments above:

Issues like #2176961: Document deprecated code that will be removed before beta 1 will needlessly break all of the patches that are removing the obsolete functions altogether.

Having to re-roll patches that remove obsolete code, because another patch "fixed" the documentation, is nothing else but a serious waste of precious contributor time, in more than one way.

I'm afraid, that makes no sense to me. → Why don't we skip the intermediate step and remove obsolete code as fast as possible instead?

I opened that issue for a practical demonstration of how this policy would work. Yes, in this case it might need some patches to be rerolled, but they should be very easy rerolls (git can probably handle them on its own). It's likely to take a lot more time to upgrade your contrib module to support the next alpha because an API was removed without warning (the old AJAX API isn't even marked deprecated yet!).

Do you value a few minutes of core contributor's time over a potentially much larger amount of contrib developer's time as they try to upgrade their modules? Early in the dev process the answer is probably yes, because there aren't really any contrib developers to worry about, but the more we encourage contrib development (with api freezes, alphas and betas) the more we need to consider their needs.

+1 #41 (again.)

It's just sloppy to fix it in the docs. That means no one knows what the API really is, which means all the time wasted is compounded by people having to answer questions about which @deprecated means what.

Every @deprecated should have a critical release blocker attached to it.

@Mile23, did you mean #31 or #41? Because they are on different sides of the argument ;). Also, by docs do you mean content on d.o, or are you including docblocks?

We seem to be discussing two basic options:
1) Remove all deprecated code ASAP.
2a) Properly document all APIs due for removal in their docblocks now (i.e. quicker than we can do option 1), then remove when we're ready.

and a variant:
2b) ...and also trigger E_USER_DEPRECATED

@ianthomas_uk:

Make deprecated code into dead code, and then remove dead code.

For example, there's no reason for variable_get() to exist in D8 core at release time, regardless of its @deprecated status. There should be change notices and examples and other ways to get help from Google so that everyone can port their stuff to CMI.

Stated another way: Even with its @deprecated tag, variable_get() must be maintained by core developers until Drupal 9 if it makes its way into Drupal 8. Which is a bigger tech debt: Gut now? Or maintain for however many years?

No one is proposing that variable_get() or any of the APIs in #2176961: Document deprecated code that will be removed before beta 1 will be included in Drupal 8.0, but we can't just delete variable_get() today.

I'm suggesting that we clearly mark these functions in the code, so it's really obvious these functions shouldn't be used any more (either by contrib or core developers). The downside of this is that it will require patches to be committed and other patches to be rerolled.

Others including sun are suggesting that we document these functions using issues on drupal.org, but not in the code. The downside of this is that it is less obvious when you're using a deprecated function and how urgently you need to replace it. Replacing removed code may also be harder if there is no release where both the old and new APIs are supported.

The patch on #2176961 could be committed now and the functions should then be considered dead for anyone planning to support 8.0. The patches to actually remove those functions still require work.

@webchick I'll work on this today. please can you have a look at the patch on #2176961: Document deprecated code that will be removed before beta 1 and say whether that's the sort of thing you are after. The document mentions replacing @deprecated with @todo, but I think we were agreed that it was better to have both. I also think its useful for internal functions to be marked @deprecated, if only for ide highlighting.

I agree with your first two points.

Yes, there should be a separate line before @see, but I wonder if it would be better to drop the @see and include the url in the @deprecated.

* @deprecated in Drupal 8.0-dev. Will be removed before Drupal 8.0-beta1, see https://drupal.org/node/1843212

I've completed all but three of those rows, because I wasn't sure what the other three should be. Where they were simple wrappers I've marked them for removal, but then I noticed check_plain was down as @deprecated rather than remove. Is that just because it's widely used, or is there a reason we can't remove it in D8?

Next step is to ensure all @todo functions have an issue listed against them.

I think we're pretty much there with this issue, with recent comments here and behaviour on other issues indicating that we're happy to rip out these functions now to make life easier as we get closer to beta (e.g. we're about to remove variable_get!).

The remaining contentious issue is whether we rip them out immediately, or mark them as deprecated first, with the main dividing line being how quickly each party estimates that we can remove the APIs. I have therefore drafted #2183187: [META] Remove or document all obsolete APIs before beta which I hope strikes a good balance - basically it says try to remove these APIs now, but if that's not easy then mark them as deprecated.

Please can people review the text of that issue, and comment back here about whether you think it's suitable or not (so we can keep that issue easy to understand for any novices we get helping). If we all agree then we can close this and move over there.

OK, to muddy the waters a little bit, alexpott/catch have been asking for the wrappers to stay in for now, to make it easier for contrib modules to upgrade.

We now have x basic options, the merits of which have been discussed above. I think we now just need a policy decision from one of the core maintainers about which option we go for (probably webchick, since she's been most involved up to now).

a) Update all docblocks now, remove when we can.
b) Update all docblocks now and removal all uses, but leave the wrapper functions.
c) Update all docblocks now, remove all uses and trigger E_USER_DEPRECATED in the functions.
d) Remove all uses ASAP, including the wrapper functions.

Arguments:
a) Extra core work, documentation only, doesn't actually get us any closer to release.
b) Friendly-ish towards contrib authors, but could lead to the functions being re-used in core.
c) Very friendly towards contrib authors, extra work, functions very easy to remove when we're ready. (E_USER_DEPRECATED should cause a core test failure).
d) Least work for core developers, least friendly for contrib authors.

FWIW I'm happy with any option except b but would favour c of them all.

My impression is webchick wants a, alexpott/catch want b, sun wants d.

d) Least work for core developers, least friendly for contrib authors.

I'd question your framing. 'Least friendly' here really means 'crystal clear.'

As far as c)... That's basically a big pile of dead code that must still be maintained, and there's still the question of the threshold at which it gets removed.

d) gives us the same effect ('PHP Fatal error: Call to undefined function' instead of E_USER_DEPRECATED), removes dead code, and doesn't punt on the issue of when to gut the code in question. Also less work in the short, medium, and long term.

Devs can then look at the handy-dandy change messages and the examples project (hint hint), and find out The New Way Of Drupal.

b) Friendly-ish towards contrib authors, but could lead to the functions being re-used in core...My impression is webchick wants a, alexpott/catch want b, sun wants d.

I don't see a comment from alexpott on this issue. Alex: if you have an opinion on this, please summarize it here.

My sense from reading the issue comments is that catch only wants the wrapper functions left in for a short time. Not to be friendly to contrib, but to be friendly to core development to allow parallel RTBC patches to be committed without breaking HEAD. @catch: please correct me if I'm misinterpreting your position. If my interpretation is correct, then a) and b) only differ by the way we split up patches, and potentially not in any other substantive way.

And it seems to me that everyone is on-board with doing d) as quickly as people manage to do it, but that since it will be a multi-month effort, we don't want d) holding up a) and b) in the meantime. So long as everyone agrees on allowing d) to happen, then I don't see any value in c), since in any situation where c) is allowed, then d) should also be allowed.

I should probably have said, my main aim is that a module written for beta1 using no deprecated APIs will run unmodified against 8.0. Probably unrealistic for reasons outside of the scope of this issue, but the closer we can get to that the better.

'Least friendly' here really means 'crystal clear.' [when contrib modules fatal error trying to use an old API]

One thing to consider here - I think it's already a given someone upgrading a contrib module after 8.0 is released won't have the benefit of this backwards compatibility layer, so why are we so bothered about it for the contrib modules that are on the bleeding edge? How many modules are we talking about? devel and config_inspector spring to mind. I think the main disadvantage of removing functions isn't to the module maintainers, but to the module users (core developers), because there will be periods where these modules fatal error when used. That's the advantage of c over d - the modules will continue to work.

The problem with d) is there's absolutely no heads-up given to module devs prior to being smacked with the fatal error.

If we're not happy with this then there are some RTBC patches that need to go back to needs work.

There are 40+ of these issues remaining to fix in total

I think you're under estimating the number of issues (the spreadsheet lists >200 functions and I think there are many more than that), but over estimating the difficultly of removing them. Either way, too much to do in a month or so. Just documenting them is going to take a long time too though, especially if they start getting removed.

I don't see a comment from alexpott on this issue.

Sorry, that was in irc. alexpott said:
"both catch-afk and I think that it is nice to leave the function around for a bit. Once it is marked as deprecated it is no longer an API change to remove it. it is nice for people upgrading modules as they get a chance to swap it instead of hard breaking. however if the function is not really used by contrib then it is okay to remove. http://drupalcontrib.org/api/drupal/7 is a good place to search for function usage. but we have not got any hard and fast rules here yet, we need to create them"

Actions

I think we need:
i) A beta blocking issue to document all deprecated APIs (every docblock must include words to the effect "will be removed before Drupal 8.0")
ii) A policy that none of the wrappers are to be removed yet.
iii) An RC (release?) blocking issue that all of the wrappers should be removed.
iv) A concerted effort to remove uses of wrappers from core.
v) (optional) A policy, started after (i) is committed, that ideally when a patch removes all uses of a wrapper, it makes that wrapper trigger E_USER_DEPRECATED. This will prevent uses creeping back into core. It's very little extra effort to add the extra line, so I think it will reduce work overall. We don't necessarily need to add these errors to all the existing wrappers, nor reject patches that don't have the errors in them.

The first part of (i) is #2176961: Document deprecated code that will be removed before beta 1. Assistance / reviews with that very appreciated. I'm just off to reroll it now. Edit: The functions listed on that issue aren't good candidates for this work at this time. I'm going to create a new issue focussing on other functions.

If we have a policy not to remove the wrappers, then that should mean a documentation patch should not conflict with the removal patches.

My sense from reading the issue comments is that catch only wants the wrapper functions left in for a short time. Not to be friendly to contrib, but to be friendly to core development to allow parallel RTBC patches to be committed without breaking HEAD. @catch: please correct me if I'm misinterpreting your position.

Yes this is exactly right. The reasoning for this was we had several patches that went from functions not being deprecated at all, to completely removed, that conflicted with other patches using the not-deprecated patches also RTBC - leaving to broken HEAD more than once a week on several occasions.

Doing that in two patches (deprecated -> remove) means that HEAD would not have got broken, or if it gets broken, it's by a much smaller, easier to review/re-roll/revert patch - since it's only removing the functions and potentially one or two stray usages that slipped in since deprecation.

I'm not adamant about this or anything, it just seemed a good idea at the time.

I'm not sure whether we're looking at the right data points here.

I'm not sure what you see, but here is what I see:

https://drupal.org/project/issues/search/drupal?status[]=Open&issue_tags... shows tremendous progress, many fixed issues, and many RTBC issues.

In fact, various contributors are very actively hammering on these issues. They are even frequently re-rolled, ready, and RTBC'ed.

The reason for why we're not making progress (← seemingly the exact counter-argument of "timely progress" being raised here) is that the RTBC patches are not committed and thus become stale. Often times, because other committed changes introduced new calls to the already @deprecated functions. Chicken and egg party! :-)

Conclusion:

1. Progress of removing @obsolete code is limited by getting patches committed in a timely fashion. Progress is not limited by contributors.
2. If even core developers are not able to update their patches for @obsolete code, then the transition time for contrib will only take much, much longer.

In essence, that's the gist of #18:

It would be a good idea to stop making intermediate changes and focus on breaking as many APIs as quickly as possible.

I can get behind #61 and doing @deprecate (with clearly defined "RIP timeline") and removal in separate patches. The consequence to that though is that every time we mark a function @deprecated and don't remove it, we are (in effect) adding a new release blocker, because of iii.

If we add E_USER_DEPRECATED to all functions, then it's a fairly trivial task to write a patch removing all of them and we won't need to go back through core re-removing calls to these functions if such a call causes a test failure.

So I don't think it's difficult to keep these functions around if we want to. We just need a decision from a core maintainer about whether they should be removed:
- ASAP (up to the patch author whether they do it using one or two patches)
- at least a release after being properly deprecated
- all together, just before release x

Note that I'm only really talking about wrapper functions. Other, non-trivial, functions are more work to maintain (or may not even work now, e.g. drupal_add_http_header), so should be removed ASAP.

RE #65, I'm going to work on a patch or patches this weekend to deprecate many functions. While this will cause some minor re-rolling, I think it's best for both the deprecation and removal efforts to continue in parallel.

I've put a first stab at a patch for (i) on #2187735: Add removal information to docblock of all @deprecated functions, does anyone fancy reviewing it?

We've made some good progress here, I'd like to revisit the actions from #59, in a slightly different order.

i) A beta blocking issue to document all deprecated APIs (every docblock must include words to the effect "will be removed before Drupal 8.0")

#2187735: Add removal information to docblock of all @deprecated functions was committed today and gets us a long way there - the vast majority of @deprecated APIs are now labelled "will be removed before Drupal 8.0". There are a few exceptions for a range of reasons: either they were internal APIs/tests that @deprecated is less appropriate for, or the @deprecated tag was added after the patch was rolled, or the @deprecated tag was debatable. We need to open issues to look at these, but they are much lower priority than the mass redocumentation patch that was just committed.

There will also need to be a range of followups to the mass patch, either because we later decide we don't want to remove a function (e.g. there was talk of keeping the entity CRUD wrappers like taxonomy_term_load), or more likely that we want to remove a function that was never marked @deprecated in the first place. The latter is a much harder problem, I've written a blog post[1] asking people to look for functions that aren't marked @deprecated, but this will probably need a more formal process before beta. Maybe we should ask the maintainers of each module to check the APIs that their modules are providing before the beta release?

iv) A concerted effort to remove uses of wrappers from core.

This is ongoing under the @deprecated issue tag and there's lots of good working happening. The sooner this can happen the better, as we don't want people to copy our bad habits, but this doesn't necessarily even need to block release now that we have these APIs properly documented.

ii) A policy that none of the wrappers are to be removed yet.
iii) An RC (release?) blocking issue that all of the wrappers should be removed.

We've generally started following a policy of not removing a function at the same time as its uses, if only because it makes the uses removal patch much less likely to break HEAD, although I think there are still patches around that do both in one.

As for when the functions actually are removed, we really just need an official policy to follow. The three basic options are "ASAP", "After a grace period" or "ALAP (as late as possible)".

The ASAP argument is that contrib should already not be using these functions and it makes life easier for core developers.
The grace period argument is that we don't want to break contrib modules that already work with 8.x by unexpectedly removing APIs.
The ALAP argument is that the easiest way to upgrade a 7.x module to support 8.x is if the APIs haven't changed. We've already broken that though by removing loads of old functions and breaking some old concepts (e.g. much reduced use of global state). See also #2042165: Add a 'deprecated' module that includes deprecated functions only when enabled.

The grace period argument only really makes sense if developers realise they are in that grace period. You could say that they are now the "will be removed before" patch has landed (or at least will be from alpha10), or we could be a bit more explicit about it by adding E_USER_DEPRECATED when a function is no longer used.

v) (optional) A policy that ideally when a patch removes all uses of a wrapper, it makes that wrapper trigger E_USER_DEPRECATED. This will prevent uses creeping back into core and make it much more obvious when contrib is using deprecated APIs.

Therefore, I feel the removal policy should either be "after alpha10", or "one release after E_USER_DEPRECATED has been added to that function".

[1] http://ithomas.name/2014/lets-get-frozen/

I don't think we can have an explicit period between deprecation and removal at the moment, it's going to be too hard to track each individual issue. For highly disruptive patches we could do that, just not everything.

Since the docs are updated I'm going to move this to beta target rather than blocker - anything that's high impact should have its own dedicated issue.

I have opened three followups:

#2205635: [meta] Review API to ensure all obsolete functions are marked @deprecated for us to catch those APIs we plan to remove that aren't already marked deprecated. If you know of any already then please mention on that issue.

#2205673: [META] Remove all @deprecated functions marked "remove before 8.0" as a meta for the removals themselves, and a reminder to review this as we approach release.

#2205685: Review @deprecated functions that are not marked "will be removed before Drupl 8.0" to take another look at the functions that were not addressed in #2187735: Add removal information to docblock of all @deprecated functions

Assuming we're happy to remove obsolete functions as soon as core doesn't depend on them, then I think we can mark this fixed.

Wording tweak (capitals to highlight changes):
"to be removed in Drupal 8.0" becomes "WILL be removed BEFORE Drupal 8.0"
"to be removed in Drupal 9.0" becomes "WILL be removed FOR Drupal 9.0"

This should make it totally clear that if you're developing for Drupal 8 you can use the latter but not the former, and if you're developing for Drupal 9 you can't use either.

I've used "before" to indicate that they might be removed at any point, and "for" to imply that they will still be supported in earlier releases (i.e. 8.x)

I've added the following section to the proposed resolution:

APIs marked "will be removed before Drupal 8.0" must help people upgrade by either:
- stating the replacement function
- linking to documentation such as a change notice
- explaining that the functionality has been removed, possibly with a link

APIs marked "will be removed for Drupal 9.0" may either use one of the above 3 option, or link to the issue in which the replacement function is being developed.

The current issue summary seems like a reasonable strategy to me. Then we just need to figure out which functions/classes/methods are which. :-)

Code which will live in Drupal until D9 is in no meaningful way deprecated.

Furthermore, not marking code as @deprecated implies that it will not change.

So basically, saying "WILL be removed FOR Drupal 9.0" is definitely clearer than the previous language, but is also incredibly misleading.

Until there is an architecture/design document for D9, and until work has begun in earnest on starting the dev process for D9, there should be no such notation. It serves no purpose, and does not fulfill any requirements (because there are no requirements).

Getting back to @Crell in #74 above, there is no design process for Drupal, so it is basically impossible to make that determination. The metric is how many criticals exist, not whether deprecated code is removed, and not whether we've met any requirements for ourselves or our users.

So therefore, literally no one in Drupal core dev is empowered to make such a decision within the dev process.

Then we just need to figure out which functions/classes/methods are which.

I tried to start that in #2205635: [meta] Review API to ensure all obsolete functions are marked @deprecated, but it really needs to be driven by someone with some authority and doesn't seem to be a high priority.

Code which will live in Drupal until D9 is in no meaningful way deprecated.

Deprecated means there's a new, better way to do something, and while the old way is still supported for backwards compatibility it should not be used when writing new code. At the time of writing I would not expect there to be much, if anything, tagged for removal in Drupal 9, but as we move closer to release it will not be acceptable to remove APIs in Drupal 8.x, so marking for removal in Drupal 9 becomes appropriate.

Until there is an architecture/design document for D9 ... there should be no such notation.

We should not be making sweeping architectural changes until there is a plan for Drupal 9, but that doesn't mean that individual APIs should not be marked as deprecated, particularly if they were replaced by new functionality in Drupal 8 but the old functionality didn't get removed for whatever reason.

Deprecated means there's a new, better way to do something, and while the old way is still supported for backwards compatibility it should not be used when writing new code. At the time of writing I would not expect there to be much, if anything, tagged for removal in Drupal 9, but as we move closer to release it will not be acceptable to remove APIs in Drupal 8.x, so marking for removal in Drupal 9 becomes appropriate.

So I agree with this.

The caveat I'd add is we've had far, far too many things marked as 'deprecated' in Drupal 8 that didn't have a viable replacement, it became synonymous with "I don't like this and would like to get rid of it one day". So, we should only be marking things in 8.x as @deprecated, will be removed in 9.x if core has been 100% converted to the new way to do things in 8.x already. That way the 9.x change should only be removing the deprecated code.

@ianthomas_uk: Deprecated means there's a new, better way to do something, and while the old way is still supported for backwards compatibility it should not be used when writing new code.

Right. And if you say something is deprecated until Drupal 9, you are also saying that you will support the backward compatibility UNTIL Drupal 9.

That is: Anything annotated this way MUST be maintained by the maintainers UNTIL Drupal 9. It is automatic technical debt which will be ongoing for years and years.

@catch: So, we should only be marking things in 8.x as @deprecated, will be removed in 9.x if core has been 100% converted to the new way to do things in 8.x already. That way the 9.x change should only be removing the deprecated code.

'Deprecated until 8.1' seems like a more reasonable promise to make, but that still leaves us with the question: What basis do we use for determining whether something adheres to the 'new way?' Is there a document that says something like, "All classes must be injectable"? Because if there is, then that's something to use for prioritizing issues.

we should only be marking things in 8.x as @deprecated, will be removed in 9.x if core has been 100% converted to the new way to do things in 8.x already.

This came up on #2319875: Deprecate all db_* functions. I totally agree that we shouldn't mark something as deprecated if we haven't got a replacement, or at least an explanation of why there won't be a replacement. I don't see why it matters if core has been converted or not. If we've written a replacement, then we are presumably expecting to remove the old version at some point (we don't want to maintain both) and therefore we want to encourage all new code to be written using the new API so we don't have to convert it at a later date.

You could argue that converting core to use a new API proves the API is complete, but that's not really true because core's use cases are different to contrib. We've seen this with the move away from drupal_get_css() etc which causes problems with the libraries module that weren't problems for core. Where we're just converting a procedural API to a similar OO API we can see if the API is complete by comparing the old functions to the new methods.

Personally I'd be rejecting patches that introduced a replacement API if they didn't also deprecate the API that they are replacing.

This issue was marked as a beta target for the 8.0.x beta, but is not applicable as an 8.1.x beta target, so untagging.

Re

This came up in my search about deprecations. This is a long read and I will admit that I have skimmed the issue. I gather it is to ensure that there is an update path for the Drupal 8 deprecations. If that is correct, then there is nothing more to do here and this can be closed.

I decided to see what deprecations, if any, from Drupal 8 to be removed in Drupal 9 were still around. I found the following in 9.4.x

core/lib/Drupal/Core/Entity/EntityStorageInterface.php

   * @todo Deprecated in Drupal 8.5.0 and will be removed before Drupal 9.0.0.
   *   Use \Drupal\Core\Entity\RevisionableStorageInterface instead.

   * @todo Deprecated in Drupal 8.5.0 and will be removed before Drupal 9.0.0.
   *   Use \Drupal\Core\Entity\RevisionableStorageInterface instead.

See #2926958: Remove revision-related methods from EntityStorageInterface

core/lib/Drupal/Core/Render/theme.api.php

 * Failure to do so is likely to result in security vulnerabilities. Theme
 * functions are deprecated in Drupal 8.0.x and will be removed before
 * Drupal 9.0.x. Use Twig templates instead.

 * vulnerabilities. Theme functions are deprecated in Drupal 8.0.x and will be
 * removed before Drupal 9.0.x. Use Twig templates instead.

* - function: (deprecated in Drupal 8.0.x, will be removed in Drupal 9.0.x)

See Change notice
See #3224178: Remove theme function documentation from theme.api.php
*/

The first one is a todo with an issue so that has a path to finish whatever should be done. For the other one, there is an issue to update the documentation. That covers those cases.

I don't see anything more to do in this issue.

Confirmed with larowlan that this can be closed now.

Thank you to everyone who helped define our deprecation policy!