Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Fetchpriority module as the functionality is now available in WordPress core #854

Merged
merged 2 commits into from Oct 11, 2023

Conversation

mukeshpanchal27
Copy link
Member

Summary

Fixes #853

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

@mukeshpanchal27 mukeshpanchal27 added [Type] Enhancement A suggestion for improvement of an existing feature [Focus] Images Issues related to the Images focus area no milestone PRs that do not have a defined milestone for release [Module] fetchpriority Issues for the fetchpriority module labels Oct 11, 2023
@mukeshpanchal27 mukeshpanchal27 self-assigned this Oct 11, 2023
@mukeshpanchal27 mukeshpanchal27 marked this pull request as ready for review October 11, 2023 06:58
@joemcgill
Copy link
Member

I understand that the rationale here is that since fetchpriority has landed in WP 6.3, that this plugin is essentially finished software and could be archived. However, we continue to have ongoing conversations about how to improve the heuristics and markup techniques used to accurately apply fetchpriority to the best LCP candidate.

For that reason, I wonder if we should keep this plugin in active development as a way of quickly iterating on those improvements prior to them being shipped in an official WP release?

@felixarntz
Copy link
Member

@joemcgill I thought about this as well before agreeing to remove the module, my conclusion is the following: Yes, we may want to improve the heuristics, but to me this does not fit into this existing module / plugin, as its goal was to enable fetchpriority support.

Improving the heuristics affects multiple attributes (loading and fetchpriority) and IMO would better fit into a separate module, potentially even as part of a "smart LCP detection" module. Even if that is focused on client-side detection, it could also provide functionality to improve the core server-side detection.

@felixarntz felixarntz added this to the PL Plugin 2.7.0 milestone Oct 11, 2023
@felixarntz felixarntz removed the no milestone PRs that do not have a defined milestone for release label Oct 11, 2023
@felixarntz felixarntz changed the title Remove Fetchpriority module and plugin Remove Fetchpriority module as the functionality is now available in WordPress core Oct 11, 2023
Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mukeshpanchal27, LGTM!

@joemcgill
Copy link
Member

and IMO would better fit into a separate module, potentially even as part of a "smart LCP detection" module

I had the very same thought when writing my original comment. I'm onboard with that plan.

@felixarntz felixarntz merged commit b5aba98 into trunk Oct 11, 2023
14 checks passed
@felixarntz felixarntz deleted the remove/853-fetchpriority-module branch October 11, 2023 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Images Issues related to the Images focus area [Module] fetchpriority Issues for the fetchpriority module [Type] Enhancement A suggestion for improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove Fetchpriority module and plugin
4 participants