Automattic / msm-sitemap

Comprehensive sitemaps for your WordPress VIP site. Joint collaboration between Metro.co.uk, WordPress VIP, Alley Interactive, Maker Media, 10up, and others.
73 stars 37 forks source link

Add filter to limit the sitemap date range #147

Closed kasparsd closed 5 years ago

kasparsd commented 5 years ago

Fixes #146.

kasparsd commented 5 years ago

This is ready for code review.

GaryJones commented 5 years ago

How about a test to demonstrate and prove how this new filter works in practice?

kasparsd commented 5 years ago

@GaryJones Are you thinking of a unit test for the filter? I'm not sure exactly what to test there since it is only a pass-through for a variable and the result depends purely on the filter implementation. In a pure unit test way it would just check if apply_filters() was called during build_root_sitemap_xml().

We've already added an example to the readme to illustrate a sample usage.

Did you have any other ideas in mind?

GaryJones commented 5 years ago

In a pure unit test way it would just check if apply_filters() was called during build_root_sitemap_xml().

Right - we're not testing how apply_filters() works, only that build_root_sitemap_xml() has the behaviour / logic such that the value is filterable.

lgedeon commented 5 years ago

I don't see any tests for build_root_sitemap_xml, but it appears that to test this I would need to:

@GaryJones, does this sound right? Also should this be added to test-sitemap-functions.php, test-sitemap-filter.php or somewhere else?