Open Flynsarmy opened 4 years ago
It would be nice to break it down to:
and paginations for all of those, where possible.
We de-duplicate at the end, so I'm less concerned about detecting too much vs not enough, if there is some double-up.
Happy for you to make the call regarding this area
Hmm. If we split this up into DetectCategoryURLs
, DetectTagURLs
and DetectCustomTaxonomyURLs
we get a lot of duplicate code and violate DRY. In fact the code for all 3 classes (and their respective tests) would be identical except for the argument passed to get_taxonomies
. This would also result in 6 detection classes to maintain instead of 2 when we include the pagination version of each.
Sidenote: In both cases we're going to need a filter for which taxonomies to crawl. We don't currently have this.
The logical breakup of built-in and custom taxonomies into separate classes makes sense, as does joining them all into a single detection class as all behavior between custom and built-in is the same so it's just a personal preference thing. We'll need to use this same philosophy for Posts and custom post types. I'm happy to go either way on this though. Let me know and I'll make it happen.
@john-shaffer do you have any thoughts to add?
I haven't looked at the detection code much, but if it's identical apart from a change of argument, then it makes sense to keep it all in one class.
A few issues here:
DetectTaxonomyPaginationURLs
orDetectCustomTaxonomyPaginationURLs
depending on the below