REQUEST_TIME documentation is misleading

Here's a patch with some new text.

+++ b/includes/bootstrap.incundefined
@@ -193,8 +193,9 @@ define('LANGUAGE_RTL', 1);
+ * $_SERVER['REQUEST_TIME'] is a float with microseconds since PHP 5.4.0, but float
+ * timestamps confuses most of the PHP functions (including date_create()). We
+ * store an integer version of this number in a constant REQUEST_TIME.

I would reverse this:

"REQUEST_TIME is an integer measuring seconds. This differs from $_SERVER['REQUEST_TIME'], which is stored as a float since PHP 5.4.0, because float timestamps confuse most PHP functions (including date_create()).

If you prefer to keep the doc as-is, then please fix a couple minor grammatical errors:

$_SERVER['REQUEST_TIME'] is a float with microseconds since PHP 5.4.0, but float timestamps confuse most PHP functions (including date_create()). We store an integer version of this number in the REQUEST_TIME constant.

3 days to next Drupal core point release.

If you are going to fix this documentation, could you also bring the entire docblock up to the standards in
http://drupal.org/node/1354 - thanks! In particular, the first line is not right.

Also I agree with the review above. :)

Yeah the new sentence is better.

I didn't see the newest comment regarding the document standard. I'll do one more (Ignore the patch attached to this comment) ;-)

So something more like this?

/*
  * Defines a short form of the request time global as an integer measured in seconds.
  *
  * This differs from $_SERVER['REQUEST_TIME'], which is stored as a float
  * since PHP 5.4.0, because float timestamps confuse most PHP functions
  * (including date_create()).
  */

That looks better, yes! The only thing that needs to be changed is that each line should wrap as close to 80 characters as possible without going over, and if the first line is longer than 80 characters, it should be shortened. It's allowable for constants' one-line summaries to start with something other than a verb (for instance, it could say just "Short form of the ..." rather than "Defines the short form...").

Shortened and updated.

I would strongly avoid "This constant is a ..." in the first line of this docblock. We don't do that with functions, and we shouldn't do that with constants either. But you're right that it's not a sentence. I guess we're back to starting with a verb like we do with functions.

Second sentence - if it's necessary, put it in.

I'll give it another go.

Should we put the entire explanation of the epoch and $_SERVER['REQUEST_TIME'] in though? Could we not just put a @see to the docs on php.net explaining it all?

I would argue that the second sentence should be there since when I read the doc initially, my first thought was "well why not just use the server time?"
The second sentence gives me a reason to use this instead since I wasn't aware of the change to the variable PHP 5.4

Definitely, link to php.net rather than trying to duplicate its explanation of server time.

And I'm in favor of the second sentence if it answered a burning question you had after reading the initial doc. That's why we write doc, to answer those burning questions. :) How about a new patch?

And by the way, this needs to be (technically) patched in 8.x first and then backported to 7.x (although at this point in the D8 dev cycle, mostly "backport" means that the patch can just be applied to D7).

Alright, so here's the updated block (third time's the charm ;-)

1) Text

/**
 * Time of the current request measured in the number of seconds since the
 * Unix Epoch
 *
 * This differs from $_SERVER['REQUEST_TIME'], which is stored as a float
 * since PHP 5.4.0. Float timestamps confuse most PHP functions
 * (including date_create()).
 *
 * @see 'http://php.net/manual/en/reserved.variables.server.php'
 * @see 'http://php.net/manual/en/function.time.php'
 */
define('REQUEST_TIME', (int) $_SERVER['REQUEST_TIME']);

The quotes around the URLs in the doc block aren't in the patch. They're just there to stop the text formatter from turning them into links.

The bit about using it as an alternative to time() I think would be better suited as a comment on the API site instead of part of the docs themselves since it is just a suggestion. In some cases the developer may want to call time() instead and we don't want to discourage that.

I put two @see references since they both discuss the $_SERVER['REQUEST_TIME'] variable.

+++ b/includes/bootstrap.incundefined
@@ -191,10 +191,15 @@ define('LANGUAGE_LTR', 0);
+ * Unix Epoch

needs a period (.)

-2 days to next Drupal core point release.

I've attached a patch with the trailing period following "Unix Epoch".

Forgot to change status.

A couple of issues here:

 define('WATCHDOG_DEBUG', 7);
-
 /**
  * @} End of "defgroup logging_severity_levels".
  */

Why remove that line? I think it belongs there for readability.

 /**
- * For convenience, define a short form of the request time global.
+ * Time of the current request measured in the number of seconds since the
+ * Unix Epo...

The first line of any docblock is one sentence less than 80 characters on one line.

+ * @see http://php.net/manual/en/reserved.variables.server.php
+ * @see http://php.net/manual/en/function.time.php

We like to avoid linking directly to the English-specific version of the PHP manual. There are other links to php.net that avoid this problem in the Core code. Please follow their example.

Patch from #16 re-rolled based on feedback in #18.

SOLD!!

Committed to 7.x and 8.x. Thanks!