Opened 2 years ago
Last modified 23 months ago
#57211 assigned enhancement
Split up `WP_Query::get_posts()`
Reported by: | costdev | Owned by: | costdev |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | |
Component: | Query | Keywords: | has-patch dev-feedback |
Focuses: | Cc: |
Description
WP_Query::get_posts() is a huge method at ~1650 lines. This makes it quite hard to follow and maintain.
By splitting it up:
- It's easier to follow.
- It's easier to maintain.
- It's easier to write tests.
- Our scroll wheels will last a little longer.
As this is possibly the most critical method in Core, the aim of this ticket is not to refactor the logic, but just to abstract functionality. Changes to logic and such could be done later, in more maintainable code.
This should significantly reduce the chance of introducing regressions, and hopefully mean that committing such a change can be done in the near future. All new methods should have private
visibility.
Change History (3)
This ticket was mentioned in PR #3667 on WordPress/wordpress-develop by @costdev.
2 years ago
#1
2 years ago
#2
I ran a coverage report to see what line coverage is like with the existing tests. Bear in mind that @covers
annotations aren't complete in the test suite and such, but this shows the code is executing and in terms of what the test methods are specifically testing, there are no failures.
get_posts()
is the only original method. From parse_meta_query()
to apply_filters_for_caching_plugins()
are the 30 new private
methods introduced in this PR. The tests haven't been modified at all.
#3
@
2 years ago
- Keywords dev-feedback added
@audrasjb @SergeyBiryukov I've tried to keep this as minimal as possible. It's a partial splitting up, and I haven't tried to refactor any logic in the process. Despite this, I understand that this kind of change would come with concerns about getting it right. That said, I think the sooner we can get this reviewed, the better, and if there's specific testing you have in mind that you'd like to be done before commit, then we can get the ball rolling on that.
Are either/both of you available to review the PR? And are there any other committers you'd like to take a look at it?
WP_Query::get_posts()
is a huge method at ~1650 lines. This makes the method hard to follow and maintain.This PR is an attempt to split this method into smaller pieces by abstracting some functionality to new
private
methods.At the moment:
WP_Query::get_posts()
is reduced from ~1650 lines to ~920 lines - roughly a reduction of 45%.private
methods have been introduced.Please note:
Props to @peterwilsoncc for skilfully convincing me that I should be the one to try this, and to @mukeshpanchal for a review of the initial abstractions.
Trac ticket: https://core.trac.wordpress.org/ticket/57211