Codecov Report

❗ No coverage uploaded for pull request base (2.x@1f61181). Click here to learn what that means.
The diff coverage is 100%.

@@          Coverage Diff           @@
##             2.x    #2135   +/-   ##
======================================
  Coverage       ?   95.72%           
  Complexity     ?     1539           
======================================
  Files          ?       50           
  Lines          ?     3949           
  Branches       ?        0           
======================================
  Hits           ?     3780           
  Misses         ?      169           
  Partials       ?        0

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 1f61181...301f0e8. Read the comment docs.

Holy moly. And here I thought dates would be simple? Thanks for your work thus far @gchtr I'll give this a spin this along with #2136 to see how things work

@gchtr this is fantastic. I LOVE the idea of just stealing Twig's tests — makes sure we match their functionality AND lazy.

Obviously, the big Q is whether to support versions prior to 5.3 in 2.x. I'm definitely partial to dropping support. Currently the WordPress.org Stats have version 5.3 at 29% after about 6 weeks of its release. I think new Timber users for 2.x (who will no longer be able to download from WP.org and have to use Composer) are highly likely to be using fresh installs with the newest version.

That said, Travis flagged a ton of errors coming from a lack of the wp_timezone_string(); function. I dropped in stub of it to see if that will restore functionality for WP 4.9.8. If a 5 min fix like that works, I'm persuadable — but I don't think the backwards compatibility right now should be a focus of our efforts.

I LOVE the idea of just stealing Twig's tests — makes sure we match their functionality AND lazy.

Riiight? It was actually quite rewarding to drop in the tests and then make them work one by one.

I think new Timber users for 2.x (who will no longer be able to download from WP.org and have to use Composer) are highly likely to be using fresh installs with the newest version.

That were my thoughts, too 👍.

If a 5 min fix like that works, I'm persuadable — but I don't think the backwards compatibility right now should be a focus of our efforts.

There would be more new functions to cover in #2133. And there’s wp_date(), which wasn’t even flagged yet. I could check whether copy-pasting these new functions into a new helper class will work to make tests run in 4.9.8.

However, apart from a couple of new functions, there apparently were a lot of changes to WordPress Core in other places. For example, they tried to reduce the amount of times mysql2date() is being used. I wouldn’t want to dare to say that Timber is compatible with WordPress < 5.3 if we only added stubs for missing functions. There could be so many other small errors. 😅

If we wanted to add support, we would probably have to work with the existing functions in WordPress < 5.3, namely date_i18n() etc. And that wouldn’t be a 5 minute fix.

cool, we're on the same page then. The idea of stubbing in that function sounded lovely, but in addition to the issues you flagged (date_i18n() and what not) — it also didn't work.

I updated Travis on this branch to not support/test WP before 5.3.2. Even though right now 5.3.2 and latest are essentially the same, this gives us a range for support going forward

@gchtr found a small issue with the tests we might want to resolve. Sometimes we'll get errors on tests like:

1) TestTimberDates::testTimeAgoWithPostDateAndCurrent
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'3 years ago'
+'4 years ago'
/home/travis/build/timber/timber/tests/test-timber-dates.php:50

function testTimeAgoWithPostDateAndCurrent() {
	$pid  = $this->factory->post->create( [
		'post_date' => '2016-07-06 20:03:00',
	] );
	$post = Timber::get_post( $pid );
	$str = DateTimeHelper::time_ago( $post->date() );
	$this->assertEquals(
		sprintf( '%s ago', human_time_diff( strtotime( $post->post_date ) ) ),
		$str
	);
}

Why? In the function we create a post created on July 6, 2016 at ~8pm (or 20:03:00 as some say). But when $post->date() sends the date/time info over it's a human-readable string of July 6, 2016: just the date w/o any time info. When this is compared alter to $post->post_date (which does have time info) you can get diff's in "X days ago" "Y years ago" b/c of the slight time diff. So we should either ...

A) Set all the time stamps to midnight so there's never a diff
B) Send time-inclusive PHP date formats to like $post->date('M j, Y g:i a') so we always get time back
C) Do nothing because this will only be an occasional occurrence.

Which of those is your favorite?

So we should either ...

A) Set all the time stamps to midnight so there's never a diff
B) Send time-inclusive PHP date formats to like $post->date('M j, Y g:i a') so we always get time back
C) Do nothing because this will only be an occasional occurrence.

Which of those is your favorite?

@jarednova I guess if we check for time diffs lower than a day (e.g. "5 hours") in other tests, then we can very well go with A).

Will you update the tests, or should I do it?

@jarednova When WordPress versions below 5.3 are not supported anymore, should we add a check when Timber initializes and provide a proper warning?

@gchtr good call! I'll make a separate issue/PR to track that

@jarednova Can you approve your requested changes and merge this? Everything seems fine from my side.

Yup, looks good — great to have this one in!!!!