Problem/Motivation

From https://www.drupal.org/project/drupal/issues/2902895#comment-13461286

@catch and @alexpott agreed in Slack to postpone removing the constant to D10 as it seems like a lot of work is left that close to the first beta deadline. We can keep working on its removal in core but should also do an issue to change the deprecation message. And I guess remove it again from the hardcoded list in phpstan-drupal?

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#12 3113284-12.patch497 byteshardik_patel_12
#11 interdiff-02-04.txt497 bytesmpdonadio
#11 3113284-04.patch624 bytesmpdonadio
#2 3113284-02.patch536 bytesmpdonadio

Comments

mpdonadio created an issue. See original summary.

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
commented
Status: Active » Needs review
Issue tags: +Needs change record updates
StatusFileSize
new 3113284-02.patch536 bytes

I'll update https://www.drupal.org/node/2785211 when this lands.

Also created https://github.com/mglaman/phpstan-drupal/pull/115

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Reviewed & tested by the community

+1, we aren't going to finish this in the D9 beta window given the complexities discovered so far.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed f32344341b to 9.0.x and 8f72af8913 to 8.9.x and 235845c4de to 8.8.x. Thanks!

Backported to 8.8.x as this is a docs fix

  • alexpott committed f323443 on 9.0.x 
    Issue #3113284 by mpdonadio: Move deprecation of REQUEST_TIME to Drupal...

  • alexpott committed 8f72af8 on 8.9.x 
    Issue #3113284 by mpdonadio: Move deprecation of REQUEST_TIME to Drupal...

  • alexpott committed 235845c on 8.8.x 
    Issue #3113284 by mpdonadio: Move deprecation of REQUEST_TIME to Drupal...
catch’s picture

catch
he/him
Primary language English
commented
Status: Fixed » Active

If we're not sure it's possible to remove all usages, should we actually undeprecate this (still with a recommendation to use the service), then re-deprecate in 9.x when we've removed all the usages?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

I think it is possible to remove all the usages. But maybe we should update the deprecation message again to say if the Drupal service layer is not present what to do. See #3104566: Implement Server Timing performance metrics for example where we use $request->server->get('REQUEST_TIME_FLOAT').

catch’s picture

catch
he/him
Primary language English
commented

So I am thinking of Berdir's comment here: https://www.drupal.org/project/drupal/issues/2902895#comment-13461259

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
commented
Status: Active » Needs review
StatusFileSize
new 3113284-04.patch624 bytes
new interdiff-02-04.txt497 bytes

I think we can remove all usages, it's just a lot of work. Not wed to the language, but I think the deprecation message can just reference the change record in it (like in FieldMigration). And then we can update the CR to

- Change Drupal 9 to Drupal 10
- Add a "Caveats" section (again, not wed to the word) that lists some situations where pulling from the $request or $_SERVER really is most appropriate, since most(?) of those usages would be edge cases the majority of developers need to use.

?

hardik_patel_12’s picture

hardik_patel_12 commented
StatusFileSize
new 3113284-12.patch497 bytes

Patch is failed to apply that's why re-applying patch.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Fixed

Just committed #3113476: Fallback when request is not available on the stack in Time service, so now that replacement is feasible, moving back to fixed.

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
commented
Issue tags: -Needs change record updates

Updated the change record, https://www.drupal.org/node/2785211

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.