Skip to content

2.x Update post.date#2137

Merged
jarednova merged 6 commits into2.xfrom
2.x-post-date
Jan 9, 2020
Merged

2.x Update post.date#2137
jarednova merged 6 commits into2.xfrom
2.x-post-date

Conversation

@gchtr
Copy link
Copy Markdown
Member

@gchtr gchtr commented Jan 2, 2020 •
edited
Loading

Ticket: #2135

Issue

As of WordPress 5.3, there we a lot of updates to the Date/Time component. Using date_i18n() is now discouraged and it might be deprecated in the future. Also, there are new helpful functions like get_post_timestamp() that we can make use of.

Solution

  • Uses functions that were newly introduced in WordPress 5.3 to get post times.
  • Removes usage of date_i18n() and tries to reduces usage of strtotime() for posts.
  • Introduces new $post->timestamp() and $post->modified_timestamp() functions to retrieve a post timestamp as the recommended way.
  • Introduces a new DatedInterface() class to streamline the usage of the timestamp(), modified_timestamp(), date(), date_modified(), time() and time_modified() functions. In a later stage, we can use this interface for the Timber\Comment class.

Impact

Breaks backwards compatibility with WordPress version below 5.3. Discussion in #2135.

Usage Changes

New $post->timestamp() and $post->timestamp_modified() functions.

Considerations

  • Might there be a better name for the DatedInterface, which says that an object has a timestamp, a publishing and a modification date?

Testing

Ran test. Fine until now.

@gchtr gchtr added the 2.0 label Jan 2, 2020
@gchtr gchtr self-assigned this Jan 2, 2020
@gchtr gchtr marked this pull request as ready for review January 2, 2020 12:52
pascalknecht
pascalknecht reviewed Jan 2, 2020
View reviewed changes
@jarednova
Copy link
Copy Markdown
Member

jarednova commented Jan 5, 2020

I love this too! Pretty short road between implementing this into Comment as well.

I think there are good arguments between HasDate vs. DatedInterface — though ultimately it's a pretty internal class that devs will not be interacting with directly very much. I think I'm slightly partial to DatedInterface b/c it's a little more explicit about its role (and could serve as a model for other interfaces, (AuthoredInterface?) that might be created in the future.

I'll let @pascalknecht and/or @acobster have a crack at formal review here, but everything I saw looked good!

acobster
acobster previously approved these changes Jan 5, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@acobster acobster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a suggestion to use the shorthand ternary operator but I'll leave it up to @gchtr as to whether to update that or not, since this has codebase-wide style implications. Personally I prefer the terser ?: syntax and I'd like to see us gradually transition to that as we go, but this is a minor concern.

Approving for now.

@gchtr gchtr changed the base branch from 2.x-twig-date-compat to 2.x January 7, 2020 20:43
@codecov
Copy link
Copy Markdown

codecov bot commented Jan 7, 2020 •
edited
Loading

Codecov Report

Merging #2137 into 2.x will increase coverage by 0.02%.
The diff coverage is 44.44%.

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #2137      +/-   ##
============================================
+ Coverage     95.65%   95.67%   +0.02%     
+ Complexity     1542     1531      -11     
============================================
  Files            50       50              
  Lines          3956     3933      -23     
============================================
- Hits           3784     3763      -21     
+ Misses          172      170       -2
Impacted Files Coverage Δ Complexity Δ
lib/Twig.php 98.91% <ø> (+1.57%) 53 <0> (-1) ⬇️
lib/Timber.php 85.96% <ø> (ø) 32 <0> (ø) ⬇️
lib/DateTimeHelper.php 100% <100%> (+12.5%) 13 <0> (-2) ⬇️
lib/Admin.php 44.44% <37.5%> (-51.56%) 3 <1> (-8)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d390c03...aad5f87. Read the comment docs.

@gchtr gchtr mentioned this pull request Jan 7, 2020
Merged
@acobster
Copy link
Copy Markdown
Collaborator

acobster commented Jan 7, 2020

@gchtr hahahaha @ "Elvis operator"! I hadn't heard that term before. Love it.

@gchtr
Copy link
Copy Markdown
Member Author

gchtr commented Jan 7, 2020

@acobster When I learnt about it, they called it Elvis operator and I first thought that this was the official name 😆🕺🏻.

@jarednova
Copy link
Copy Markdown
Member

jarednova commented Jan 9, 2020

Took care of a few merge conflicts, all good now!

@jarednova jarednova merged commit 8e61ab8 into 2.x Jan 9, 2020
@jarednova jarednova deleted the 2.x-post-date branch January 12, 2020 20:41
@gchtr gchtr mentioned this pull request Apr 11, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@acobster acobster acobster left review comments

+1 more reviewer

@pascalknecht pascalknecht pascalknecht left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@gchtr gchtr

Labels

2.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gchtr @jarednova @acobster @pascalknecht