Make WordPress Core

Opened 8 months ago

Closed 6 months ago

Last modified 6 months ago

#60620 closed defect (bug) (fixed)

Twenty Twenty-Four: Remove pattern from home template to improve performance

Reported by: onemaggie's profile onemaggie Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.6 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch commit
Focuses: Cc:

Description (last modified by poena)

This PR removes the pattern from the home template and instead replaces it directly with its contents.

This was introduced in [this PR](https://github.com/WordPress/twentytwentyfour/pull/486). Creating the pattern was needed to solve what the PR was stating (we need the Home Business pattern to show on the template replacement flows) but we don't need to use it in the template, and the code repetition (which is minor) is better than having to do a an extra pattern replacement.

To test this PR, check that the home page is unchanged and that creating a new template still shows the home page pattern (by creating a template for the front page, for example)

Change History (15)

This ticket was mentioned in PR #6170 on WordPress/wordpress-develop by @onemaggie.


8 months ago
#1

  • Keywords has-patch added

This PR removes the pattern from the home template and instead replaces it directly with its contents.

This was introduced in this PR. Creating the pattern was needed to solve what the PR was stating (we need the Home Business pattern to show on the template replacement flows) but we don't need to use it in the template, and the code repetition (which is minor) is better than having to do a an extra pattern replacement.

To test this PR, check that the home page is unchanged and that creating a new template still shows the home page pattern (by creating a template for the front page, for example)

https://github.com/WordPress/wordpress-develop/assets/3593343/b2e99f9d-6a58-4de3-aa27-168e33e1884d

Trac ticket:
https://core.trac.wordpress.org/ticket/60620

#2 @onemaggie
8 months ago

@youknowriad @richtabor We discussed this, if you would have a look at the patch I'd appreciate it

#3 follow-up: @poena
8 months ago

Please include a ticket description and a summary of the mentioned discussion.

#4 in reply to: ↑ 3 ; follow-up: @onemaggie
8 months ago

Replying to poena:

Please include a ticket description and a summary of the mentioned discussion.

I didn't add it to the ticket because the bot already copies from GH, is it better to duplicate? I never know what is the way to go

#5 @onemaggie
8 months ago

  • Description modified (diff)

#6 @onemaggie
8 months ago

  • Description modified (diff)

#8 in reply to: ↑ 4 @afercia
8 months ago

Replying to onemaggie:

Replying to poena:

Please include a ticket description and a summary of the mentioned discussion.

I didn't add it to the ticket because the bot already copies from GH, is it better to duplicate? I never know what is the way to go

Please refer tp the contributors handbook at https://make.wordpress.org/core/handbook/contribute/#submitting-tickets

All contributors should clearly describe what a ticket is about, even if it's a ticket meant to accompany a pull request submitted on the GitHub repository. Tickets should precede pull requests or patches. That's important in a collaborative open source project to:

  • Allow broader discussion on the proposed change.
  • Document the history of the project: Trac represents the history of WordPress. Ten years from now, anyone should be able to search Trac to understand why and when a change was made.

@youknowriad commented on PR #6170:


7 months ago
#9

This is looking good to me, probably best to commit it after 6.5

@onemaggie commented on PR #6170:


6 months ago
#10

Now is after 6.5, should we merge this?

#11 @youknowriad
6 months ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 6.6

#12 @SergeyBiryukov
6 months ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 57945:

Twenty Twenty-Four: Remove pattern from home template to improve performance.

Creating the Home Business pattern was needed to show it on the template replacement flows, but there's no need to use it in the template, and the minor code repetition is better than having to do a an extra pattern replacement.

Follow-up to PR #486.

Props onemaggie, youknowriad, poena, afercia.
Fixes #60620.

@SergeyBiryukov commented on PR #6170:


6 months ago
#13

Thanks for the PR! Merged in r57945.

#14 follow-up: @swissspidy
6 months ago

How does this actually improve performance? Are there any numbers?

#15 in reply to: ↑ 14 @onemaggie
6 months ago

Replying to swissspidy:

How does this actually improve performance? Are there any numbers?

By not having to resolve the pattern, the template loads faster, but we haven't measured by how much. Since the pattern isn't serving any function (utility or otherwise) there is no reason for the extra step

Note: See TracTickets for help on using tickets.