Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
API page: http://api.drupal.org/api/drupal/includes--bootstrap.inc/constant/REQUES...
Describe the problem you have found:
REQUEST_TIME is a float with microseconds...
No. $_SERVER['REQUEST_TIME']
may be a float, but the REQUEST_TIME
described on this page is an int. Simply in seconds, not microseconds.
Comment | File | Size | Author |
---|---|---|---|
#19 | request-time-1289208-19.patch | 898 bytes | drupal_was_my_past |
#16 | 1289208-RequestTime-16.patch | 1.08 KB | skottler |
#14 | D8-1289208-RequestTime-15.patch | 902 bytes | minorOffense |
#8 | 1289208-RequestTime-5039386.patch | 917 bytes | minorOffense |
#5 | 1289208-RequestTime (Corrected).patch | 860 bytes | minorOffense |
Comments
Comment #1
Dave Cohen CreditAttribution: Dave Cohen commentedComment #2
minorOffense CreditAttribution: minorOffense commentedHere's a patch with some new text.
Comment #3
bleen CreditAttribution: bleen commentedI 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.
Comment #4
jhodgdonIf 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. :)
Comment #5
minorOffense CreditAttribution: minorOffense commentedYeah 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) ;-)
Comment #6
minorOffense CreditAttribution: minorOffense commentedSo something more like this?
Comment #7
jhodgdonThat 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...").
Comment #8
minorOffense CreditAttribution: minorOffense commentedShortened and updated.
Comment #9
Dave Cohen CreditAttribution: Dave Cohen commentedTo my reading, "short form of the request time global as an integer measured in seconds," is not really a proper sentence. How about...
This constant is the time of the current request measured in the number of seconds since the Unix Epoch (January 1 1970 00:00:00 GMT).
And perhaps add a sentence....
It is often preferable to use REQUEST_TIME rather than time(), because it is more performant, and data with the same REQUEST_TIME can be correlated to the same request.
Comment #10
jhodgdonI 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.
Comment #11
Dave Cohen CreditAttribution: Dave Cohen commentedAgreed we should keep the doc style consistant. I feel "measured in seconds" is not really informative, but "the number of seconds since the Unix Epoch..." means something. And people should know this constant is an alternative to calling time(). I leave it up to you guys how to put that in words.
Although to tell you the truth, all my experience is on apache on linux where I'm pretty sure $_SERVER['REQUEST_TIME'] is seconds sinch the Unix Epoch. I have no idea whether other servers use the same value. I'm looking at you, IIS.
Comment #12
minorOffense CreditAttribution: minorOffense commentedI'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
Comment #13
jhodgdonDefinitely, 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).
Comment #14
minorOffense CreditAttribution: minorOffense commentedAlright, so here's the updated block (third time's the charm ;-)
1) Text
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.
Comment #15
bleen CreditAttribution: bleen commentedneeds a period (.)
-2 days to next Drupal core point release.
Comment #16
skottler CreditAttribution: skottler commentedI've attached a patch with the trailing period following "Unix Epoch".
Comment #17
skottler CreditAttribution: skottler commentedForgot to change status.
Comment #18
jhodgdonA couple of issues here:
Why remove that line? I think it belongs there for readability.
The first line of any docblock is one sentence less than 80 characters on one line.
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.
Comment #19
drupal_was_my_past CreditAttribution: drupal_was_my_past commentedPatch from #16 re-rolled based on feedback in #18.
Comment #20
bleen CreditAttribution: bleen commentedSOLD!!
Comment #21
Dries CreditAttribution: Dries commentedCommitted to 7.x and 8.x. Thanks!