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: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Component%21Date...
> string $format: A format string using either PHP's date().
using either PHP's date() or...?
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | interdiff-2932617-8-11.txt | 493 bytes | voleger |
| #11 | datetime-format-2932617-11.patch | 949 bytes | voleger |
| #8 | datetime-format-2932617-8.patch | 636 bytes | bdlangton |
Comments
Comment #3
sahil432 CreditAttribution: sahil432 as a volunteer and at Publicis Sapient commentedComment #4
sahil432 CreditAttribution: sahil432 as a volunteer and at Publicis Sapient commentedAlter comment "A format string using either PHP's date() or PHP's DateTime()"
Comment #5
bdlangton CreditAttribution: bdlangton commentedLooks good
Comment #6
alexpottI think we should copy PHP's documentation and go for
These the docs from http://php.net/manual/en/datetime.format.php which is the method called here. We might want an
@see http://php.net/manual/function.date.phptoo.Comment #7
joachim CreditAttribution: joachim commentedThere is no need for a @see with a link to php.net. Our API module automatically turns a PHP function into a link, see https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Component%21Date...
Comment #8
bdlangton CreditAttribution: bdlangton commentedSimply updated the comment to match PHP's documentation as @alexpott suggested.
Comment #9
amietpatial CreditAttribution: amietpatial as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commented@bdlangton #8 works fine for me
Comment #10
borisson_@amietpatial uploading screenshots like that isn't helpful, we can see from the testbot that the patch applies.
The patch in #8 looks solid but it doesn't add the
@seeproposed by @alexpott in #6.Comment #11
volegerChanges regarding #10
Comment #12
borisson_Remakrs in #6 are fixed.
Comment #13
alexpottCrediting myself for a review comment that influenced the final patch and @joachim for opening the issue.
@amietpatial as @borisson_ said screenshots are not useful. Thanks for looking at the patch though. Posting screenshots of your codebase is not helpful, since the automated testing infrastructure tells us whether the patch applies correctly. So, I've removed your credit for this issue. In the future, you can get credit for issues by reading the issue to understand its purpose, and posting your review or testing of that purpose.
What we do need people to review is whether the issue has a correct scope, whether it passes the core gates, whether the solution completely fixes the problem without introducing other problems, and whether it's the best solution we can come up with. See the patch review guide for more information.
Comment #14
alexpottIn #7 @joachim pointed out the @see is not necessary so committing #8.
Committed and pushed 1ae69451ad to 8.6.x and b4b9137b90 to 8.5.x. Thanks!