Opened 6 months ago
Last modified 6 days ago
#50787 accepted task (blessed)
Consolidate the logic for displaying WP & PHP compatibility messages for themes
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.7 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Site Health | Keywords: | has-patch needs-testing has-unit-tests |
Focuses: | administration | Cc: |
Description (last modified by )
Background: #48491
As part of the changes on [48636-48638,48640], these messages are duplicated in at least seven different places:
- "This theme doesn’t work with your versions of WordPress and PHP. Please update WordPress, and then learn more about updating PHP."
- "This theme doesn’t work with your version of WordPress. Please update WordPress."
- "This theme doesn’t work with your version of PHP. Learn more about updating PHP."
See comment:36:ticket:48491 for the list of relevant areas.
It would be nice to consolidate the logic in one place.
Attachments (2)
Change History (20)
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 months ago
#6
@
4 months ago
- Keywords has-patch dev-feedback needs-testing added; needs-patch removed
50787.diff is an initial pass at this ticket.
This ticket was mentioned in Slack in #core by afragen. View the logs.
3 months ago
#8
@
3 months ago
A couple of code review notes:
Function naming
For the new function name, it's not clear to me what the function will do when invoked. I can infer. But I'm wondering if it would be more clear if action-based such as
wp_get_compatibility_string
to tell us that it "gets" and returns a single string. What do you think @afragen ?
Performance
Each time the function is called, the $strings
array rebuilds. This means each __()
has to run again, ie there are 12 of them. Is this necessary? Do we need to retranslate and rebuild the array each time the function is invoked?
We can use memoization for a tiny peformance boost.
#9
@
3 months ago
Revising my performance note above. See slack conversation.
The last 3 string elements are dynamic, i.e. build from the injected $name
. if no name is passed to the function, then we can skip building those last 3 elements.
For the non-dynamic strings, we can leverage memoization to generate the $strings
array once and then reuse the same array each time the function is invoked.
It's a tiny tiny performance boost.
#10
@
3 months ago
@SergeyBiryukov Can you please do a final review on the latest patch Andy and I did? What do you think?
This ticket was mentioned in PR #607 on WordPress/wordpress-develop by hellofromtonya.
3 months ago
- Keywords has-unit-tests added
Trac ticket: https://core.trac.wordpress.org/ticket/50787
Props: @afragen
#13
@
3 months ago
- Type changed from enhancement to task (blessed)
Thanks for the patch!
Didn't get a chance to review this in time for 5.6 Beta 1, but I think it can still go in before RC, since it's more of a cleanup than new functionality. Converting to a task for now.
This ticket was mentioned in Slack in #core-site-health by clorith. View the logs.
2 months ago
#15
@
2 months ago
- Milestone changed from 5.6 to 5.7
Didn't get a chance to review this in time for 5.6 RC, moving to the next release for now.
Per Sergey during today's scrub: