Implement PSR-20 in datetime.time service

kim.pepper → made their first commit to this issue’s fork.

Need to add the composer dependency.

I'm wondering if we don't just add symfony/clock and replace usages of datetime.time service? 🤔

I'm not sure symfony/clock could replace getRequestTime().

If we are adding Psr/Clock as a dependency, then there should be at least one instance where we swap out to use it.

For #9

prudloff → changed the visibility of the branch 3501375-implement-psr-20 to hidden.

prudloff → changed the visibility of the branch 3501375-implement-psr-20 to active.

Do you mean something like this?

Didn't review as it seems @kimpepper already did.

Am I correct to think we can break BC in the LinearHistory constructor because it is marked as internal and experimental?

Added a CR and updated the IS. I think this is ready.

I was creating a follow up issue to replace TimeInterface->getCurrentTime() with ClockInterface->now()->getTimestamp() in the rest of core.

I searched for usages of \Drupal::time()->getCurrentTime() and found 28 instances.

\Drupal::time() returns \Drupal\Component\Datetime\TimeInterface but this doesn't extend Psr\ClockInterface, so it's not a straight method swap.

I think we need a new \Drupal::clock() method that returns Psr\ClockInterface. This would make it easier to swap out usage of \Drupal::time()->getCurrentTime() with \Drupal::clock()->now()->getTimestamp().

We might want to do #17 and deprecate \Drupal::time()->getCurrentTime() in a follow up.

I created a follow-up 📌 [PP-1] Deprecate TimeInterface::getCurrentTime() and replace with ClockInterface::now()->getTimestamp() Postponed so back to RTBC.

I also squashed the commits and rebased on the latest 11.x. This will help if we need to continually rebase on 11.x until it gets committed.

kim.pepper → changed the visibility of the branch 3501375-symfony-clock to hidden.

Left one nitpick.

I'm trying an alternative approach in https://git.drupalcode.org/project/drupal/-/merge_requests/11533 however I can't tell if test fails are related or not? 🤔

Instead of adding onto the Time class, I'm creating a dedicated Clock implementation.

Unrelated, see 🐛 [random test failure] ImageUrlProviderTest::testResize Active

Rebased on 11.x

Updated title and issue summary to match current MR implementation approach.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Rebased on 11.x

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

kim.pepper → closed merge request !11123

Rebased on 11.x Tests green again.

According to https://www.drupal.org/about/core/policies/core-dependency-policies/depe... → we need framework and release manager signoff on adding a new dependency on psr/clock. 🤷

According to the page linked in #34 we also need to add a dependency evaluation to the issue summary. I've inserted the headings for this. Considering this is only an interface and provided by php-fig I'm not sure it's all that relevant. Have added a bit of info to the headings.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Rebased on 11.x.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Rebased on latest 11.x.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

I rebased the MR.

Hiding files

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

I feel like we need more support for this issue, otherwise we'll be endlessly rebasing it.

prudloff → changed the visibility of the branch 11.x to hidden.

I rebased the MR but I agree it is annoying having to rebase every time something changes in composer.lock.

One way to avoid the rebases would be a separate issue to add the dependency to composer, we've done that for a quite a few things in the past year or so for the same reason.

As suggested by @catch I've split off adding the composer dependency in ✨ Add PSR-20 Clock Interface as composer dependency Active and postponed this issue on that. Moved the dependency evaluation details to the other issue.