Add comment / comment id param to get_comment_time

[spacedmonkey]

Bring get_comment_time inline with get_comment_date and add a comment / comment id param to pass comment id into the function.

[travisaxton]

I've submitted a possible solution for the 'get_comment_time' function. This is my first submission for WordPress Core so I apologize if I have made any mistakes with the Trac workflow.

[SergeyBiryukov]

Thanks for the patch! Some notes:

• The new parameter should be added at the end. Adding it after $format matches get_comment_date(), but is not feasible in practice, as it would break backward compatibility.
• The DocBlock should be aligned per the ​documentation standards and would also need a ​@since note.
• comment_time() would probably need a similar change for consistency.

[travisaxton]

Thanks for the guidance and the feedback!

I updated 'get_comment_time' to reflect the changes you mentioned. Still trying to figure out the best way to change 'comment_time' for consistency. Attaching an updated patch file to make sure I am on the right track.

Thanks very much for your patience in helping me navigate the coding standards.

[travisaxton]

Version 2.0

Check if function get_comment_id is null before executing it.

Trac ticket: https://core.trac.wordpress.org/ticket/52322

[D SIGNED]

Hi,

I've just submitted a #PR2646 to check if get_comment_id is not null to avoid an error when executing.

(It's my first contribution)

[audrasjb]

Thanks for the PR @d-signed! I added two feedbacks in your PR :)

[audrasjb]

Moving for 6.2 consideration.

[rudlinkon]

Replying to SergeyBiryukov:

Thanks for the patch! Some notes:

• The new parameter should be added at the end. Adding it after $format matches get_comment_date(), but is not feasible in practice, as it would break backward compatibility.
• The DocBlock should be aligned per the ​documentation standards and would also need a ​@since note.
• comment_time() would probably need a similar change for consistency.

Yes, I agree with @SergeyBiryukov that we need a similar change in comment_time() for consistency. Also, we can add the same extra parameter $comment_ID on the get_comment_time filter so that developer can get a direct comment id instead of getting it from the $comment object.

[h4l9k]

I submitted a new patch for this 52322.3, refreshing the one from @travisaxton (new comparison against trunk) and with the additions mentioned by @rudlinkon and @SergeyBiryukov. I added $comment_ID to comment_time() for consistency and to the filter get_comment_time as well, and @since notes in both comment_time() and get_comment_time().

This is my first contribution.

[mukesh27]

This ticket was discussed in the recent bug scrub.

Thanks @spacedmonkey for the ticket!

@d-signed There are a few reviews that need to be changed.

Props to @costdev

[spacedmonkey]

There needs to be a check to see if the comment exists.

 $comment = get_comment( $comment_ID );

A check like this

$comment = get_comment( $comment_id );

        if ( ! $comment ) {
           return '';
        }

We may even need to do this

if ( !$comment || 'trash' === get_post_status( $comment->comment_post_ID ) ) {

[costdev]

This ticket was discussed during the bug scrub. This looks ready for commit consideration. Adding the appropriate keyword. JB agrees that the suggested docs change on the PR can be made during commit.

Additional props: @audrasjb

I added one commit to the PR to fix a WPCS issue

[audrasjb]

In 55284:

Comments: Allow to pass $comment_ID parameter to get_comment_time().

Props spacedmonkey, travisaxton, SergeyBiryukov, d-signed, audrasjb, rudlinkon, h4l9k, mukesh27, costdev.
Fixes #52322.

committed in https://core.trac.wordpress.org/changeset/55284

[SergeyBiryukov]

In 55287:

Comments: Allow to pass $comment_ID parameter to comment_time().

This brings consistency with:

• get_comment_time()
• get_comment_date()
• comment_date()

Includes:

• Correcting the @since tag for get_comment_time().
• Synchronizing parameter description between get_comment_date() and get_comment_time().

Follow-up to [55284].

See #52322.