Problem/Motivation

This is a followup to #2457345: Remove unnecessary formatDate call for 'custom' format & code cleanup. which has a fix which is now outdated but also contain some documentation changes. This issue is for the documentation changes.

Steps to reproduce

Proposed resolution

Expand the @param documentation for the first argument and change the name of the first argument to $type to match the calling function.

Remaining tasks

Patch
Review
Commit

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#2 3295658-2.patch1.99 KBquietone

Comments

quietone created an issue. See original summary.

quietone’s picture

quietone commented
Issue tags: +Bug Smash Initiative
StatusFileSize
new 3295658-2.patch1.99 KB

Adding bugsmash tag because I got here doing bugsmash triage.

Uploading the patch from the original issue, with a few tweaks. Since this is primarily documentation I am not starting the test. That can be done once there is agreement on the text.

quietone credited alexpott.

quietone credited tstoeckler.

quietone credited vijaycs85.

quietone’s picture

quietone commented

Adding credit

quietone’s picture

quietone commented
Status: Active » Needs review

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave commented

The changes make sense to me.

Running 10.1 tests to make sure nothing breaks.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

This issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. We are working to keep the size of Needs Review queue [2700+ issues] to around 200, following Review a patch or merge require as a guide.

Issue and title appear to match what is being fixed in the patch.
Ran the patch against 10.1 and all green
The description updates make sense to me. The additional info is definitely an improvement.

xjm’s picture

xjm
she/her
Primary language English
commented
Title: Improve documenation for DateFormatter::dateFormat » Improve documentation and parameter naming for DateFormatter::dateFormat()

  • xjm committed f38816ed on 10.1.x 
    Issue #3295658 by quietone, smustgrave, alexpott, tstoeckler, vijaycs85...

  • xjm committed 66d0a88f on 10.0.x 
    Issue #3295658 by quietone, smustgrave, alexpott, tstoeckler, vijaycs85...

  • xjm committed 418c043e on 9.5.x 
    Issue #3295658 by quietone, smustgrave, alexpott, tstoeckler, vijaycs85...
xjm’s picture

xjm
she/her
Primary language English
commented

I reviewed this with git diff --color-words, and also read the class to ensure that all usages of the parameter were replaced and that the rename made sense. It corresponds to the $type parameter of DateFormatterInterface::format() and has matching documentation.

The state in HEAD is misleading because DateFormatterInterface::format() also has a $format parameter, which is different:

   * @param string $format                                                      
   *   (optional) If $type is 'custom', a PHP date format string suitable for   
   *   input to date(). Use a backslash to escape ordinary text, so it does not 
   *   get interpreted as date format characters.

👍

Committed to 10.1.x, and cherry-picked to 10.0.x and 9.5.x. Thanks!

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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