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.A Time Service was added in Drupal 8.3 and the REQUEST_TIME constant is deprecated, to be removed by Drupal 9.0
Change record: https://www.drupal.org/node/2785211
Core service doc: https://api.drupal.org/api/drupal/core%21core.services.yml/service/datet...
| Comment | File | Size | Author |
|---|---|---|---|
| #22 | 3032131-22.request_time.patch | 35.46 KB | jonathan1055 |
| |||
Comments
Comment #2
Satyanarayan Reddy CreditAttribution: Satyanarayan Reddy commentedReplace deprecated REQUEST_TIME
Comment #4
Satyanarayan Reddy CreditAttribution: Satyanarayan Reddy commentedTime Service Added and REQUEST_TIME deprecated
Comment #5
thallesThanks for the patch!
I think we should expect the issue # 2705499 to be finalized.
Please make dependency injection when possible:
Comment #6
thallesComment #7
Satyanarayan Reddy CreditAttribution: Satyanarayan Reddy commentedHi Thalles,
Any issue in this patch
Comment #8
thallesHi Satyanarayan,
I think we should expect the issue #2705499: Replace drupal_set_message() with an injectible service to be finalized and Drupal is asked for dependency injection.
Comment #9
idebr CreditAttribution: idebr at ezCompany commentedClosed #3034226: Replace deprecated REQUEST_TIME on scheduler_api_test/scheduler_api_test.module as a duplicate issue.
Comment #10
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedThe patch needs a re-roll. Also I think we should consider creating a property
requestTimeonSchedulerBrowserTestBasebecause the change affects 12 different test classes, all of which extendSchedulerBrowserTestBase. Does it make sense to do this once and use$this->requestTimein the tests, rather than call\Drupal::time()->getRequestTime()in each of the 12 test files?Comment #11
idebr CreditAttribution: idebr at ezCompany commented\Drupal\scheduler\SchedulerManager\Drupal\Tests\scheduler\Functional\SchedulerBrowserTestBaseComment #12
idebr CreditAttribution: idebr at ezCompany commentedReroll after #3031199: Replace assertRaw(t()) with assertText() was committed.
Comment #13
thallesPatch applied with success!
Comment #14
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedThanks. However, the following two issues are more urgent as they are currently causing test failures in Travis.
#3034805: Using setPublished() with a $published parameter is deprecated in 8.3
#3034986: Replace deprecated SafeMarkup::checkPlain()
Both of these should be done first, so that we get clean test runs. Other patches may need a re-roll, so let's concentrate on these two before any others.
Comment #15
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedLinked to parent issue.
Comment #16
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedUn-postponing now that the following are in:
#3034805: Using setPublished() with a $published parameter is deprecated in 8.3
#3034986: Replace deprecated SafeMarkup::checkPlain()
Comment #17
idebr CreditAttribution: idebr at ezCompany commentedRerolled against 8.x-1.x
Comment #18
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedNeeded a re-roll after recent commits.
Comment #20
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented#19 was a random unrelated failure regarding
mkdir()Comment #21
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedI have simplified the testing code, rather than storing the time service and calling
$this->time->getRequestTime()every time, we now store the request time numeric value as a property of SchedulerBrowserTestBase via$this->requestTime = $this->container->get('datetime.time')->getRequestTime(). Then each test can simply use$this->requestTimein every place that we had$REQUEST_TIME@idebr I'd be interested to hear your opinion on this. The tests work, and it would appear to be more efficient than calling the method many times. I have not changed your code in SchedulerManager, only the tests.
The other (minor) change is in .module where the value is now stored and reused instead of calling the method three times.
Comment #22
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedRe-rolled patch #21 due to the commit in #2867271: assertEqual() is deprecated in Drupal 9. Replace with assertEquals()
Comment #24
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedThanks idebr, Satyanarayan Reddy, thalles. Commited.