Replace calls to lock() with \Drupal::lock()

Changed lock() to Drupal::lock()

#1: 2044367_2.patch queued for re-testing.

Changed lock() to Drupal::lock()

As we replaced lock() function in all places we think it makes sense to remove it from bootstrap.inc. Patch attached.

+++ b/core/includes/bootstrap.inc
@@ -3015,70 +3015,3 @@ function drupal_check_memory_limit($required, $memory_limit = NULL) {
-/**
- * @defgroup lock Locking mechanisms
- * @{
- * Functions to coordinate long-running operations across requests.
- *
- * In most environments, multiple Drupal page requests (a.k.a. threads or
...
- */

Hm. I wanted to RTBC, but what do we do with this (helpful) documentation group?

It does not fit into any of the interfaces or classes in Drupal\Core\Lock, as these docs are rather bird-level.

Normally, in PHP components/libraries, each component ships with an own README.md file, so as to be able to provide high-level anatomy and usage docs.

Perhaps we should start to do that for core components, too?

I.e., simply move the phpDoc lines into a new README.md or README.txt file in /core/lib/Drupal/Core/Lock?
(I'd prefer .md, but I don't think we have any .md files at this point)

We've saved documentation with attached patch :)

i would love sun's proposal (having those info in README.md in Drupal/Core/Lock) but then i dont think those info would appear to api.drupal.org? so lets have it in \Drupal::lock() for now.
Patch needs reroll

Reroll.

Changed status.

That documentation isn't on the function, it is a @defgroup. There is no need to remove that and move it to a specific function, I think they can live anywhere in a file that is parsed? We should rather make sure that all relevant interfaces and methods are part of that defgroup.

See https://api.drupal.org/api/drupal/core%21includes%21bootstrap.inc/group/...

That is kind of sad right now :) That should eventually list the interface, \Drupal::lock() method and the service, once we can handle them in a useful way.

@defgroups can indeed live anywhere.

If they don't have a good home near some related functions, a good idea is to put them into a .api.php file in the system.module directory, or maybe in core itself?. We discussed on some other issue recently making a catch-all core.api.php file for such topics, but I don't think it's been done.

Note: To add something to a group/topic, add @ingroup lock to it.

Maybe like this, move the @defgroup to the interface file? Also added ingroups and verified on a local installation, see screenshot.

I think moving this to the interface file is OK... technically if you have a @defgroup @{ you should also have a @} at the end, though if the file ends that would be implied. Keeping the @defgroup next to the class that is included is probably a good idea.

https://drupal.org/node/1354#defgroup

Sure, I didn't even notice the @{ there, I guess would also remove that, but this works too.

Anything else left to do here?

I thought about creating a draft notice, but I'm actually not sure, because we do https://drupal.org/node/1525118, so it might be better to just update that one. I could create an additional one for the 8.x -> 8.x change, but not sure if it's worth it.

The @{ says "Everything between this and the @} are part of the documentation of this topic and/or are things to be included", so you do need that.

IMO, editing an existing change notice that is no longer accurate is definitely a good idea, so people don't find the older change notice and get misleading information on how to update from 7 to 8. I would suggest putting language in there like "Later in the course of Drupal 8 development, these deprecated functions were removed: ...".

For people subscribing to change notices RSS (there is an RSS feed, although I have no idea if anyone subscribes), you can also create a very short change notice saying something like "During the Drupal 8 cycle, the function ... that had previously been deprecated was removed. See ... for more details."

@jhodgdon is right about the closing @defgroup tag, but I don't think it makes sense to add one to the end of the interface file, because PSR-N standards are enforcing already that a single file only contains a single interface/class/trait.

Even though I'd vastly prefer README.md files in each component (the de-facto standard for PHP components/libraries), I like the idea of moving our former high-level @defgroup documentation onto the primary interfaces.

I wonder whether we could simplify this some more and bake the @defgroup right into the interface?

/**
 * Defines the interface for lock backends.
 *
 * @defgroup lock Locking mechanisms
 *
 * [Extensive group description here]
 */
interface LockBackendInterface {
}

In other words: Skipping the opening and closing @{ and @} phpDoc grouping tags altogether.

The idea in #22 will not work without rewriting the way defgroups work in the API module, which I would prefer not to do please. @defgroup is not supported at all (nor anything like it) by the PSR-5 standard, in spite of my asking politely that they consider having this... they are not open to revisions apparently. We have had defgroup docs done the way we are doing them now for ages...

There isn't really a problem with leaving the @{ open except for editors possibly complaining about unclosed parens. Alternatively you can close the @} at the end of the defgroup documentation, and then put @ingroup into the interface docs. But please do not try to combine them. Thanks!

so, something like that :)
also made the test to use $this->container rathen than \Drupal global

That placement of the defgroup works for me. I think some people didn't want it in the same file as the interface itself, but I personally don't mind.

I have updated the existing change record, see https://drupal.org/node/1525118/revisions/view/2355494/6937569. Although the example is procedural, I've assigned the lock service to a variable so the same code applies in objects.

I've previously been told by a core maintainer (webchick IIRC) that change records are not intended for 8.x-dev to 8.x-dev upgrades and should just give the instructions for upgrading from 7.x. I think this makes sense, and provides the most readable change records for the people who are most likely to use them. If anyone is subscribing to them using RSS, then I'd suggest they switch to looking at the commit log instead.

I've rolled a new patch that keeps the lock() function itself (the current preference is that functions should be removed in simple patches to minimise the risk of HEAD breaking). I've also changed the documentation to use a $lock variable instead of \Drupal::lock(), again so the code applies to both procedural and OO code.

sounds good, lets do it

Re-roll.

Reroll is good, was just context conflicts with the recent cache() and @deprecated patches.

Looks like this is covered in the change notice at https://drupal.org/node/1525118

Committed and pushed to 8.x. Thanks!