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.

Sign up for GitHub

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

Jump to bottom

Add Speculative Loading generator tag #1102

Merged
merged 2 commits into from
Mar 29, 2024
Merged

Add Speculative Loading generator tag #1102

merged 2 commits into from
Mar 29, 2024

Conversation

felixarntz
Copy link
Member

Summary

Adds a generator tag to the Speculative Loading plugin, similar to how the other standalone plugins include one.

Relevant technical choices

@felixarntz felixarntz added [Type] Enhancement A suggestion for improvement of an existing feature [Plugin] Speculative Loading Issues for the Speculative Loading plugin (formerly Speculation Rules) labels Mar 28, 2024
@felixarntz felixarntz added this to the speculation-rules 1.1.0 milestone Mar 28, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 28, 2024 •
edited
Loading

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: felixarntz <[email protected]>
Co-authored-by: westonruter <[email protected]>
Co-authored-by: thelovekesh <[email protected]>
Co-authored-by: mukeshpanchal27 <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@westonruter
Copy link
Member

Maybe take this as an opportunity to align the other functions with this one? Namely, the constant checks seem to be a vestige of the module past.

performance/plugins/dominant-color-images/hooks.php

Lines 180 to 187 in 71055b3

function dominant_color_render_generator() {
if (
defined( 'DOMINANT_COLOR_IMAGES_VERSION' ) &&
! str_starts_with( DOMINANT_COLOR_IMAGES_VERSION, 'Performance Lab ' )
) {
echo '<meta name="generator" content="Dominant Color Images ' . esc_attr( DOMINANT_COLOR_IMAGES_VERSION ) . '">' . "\n";
}
}

performance/plugins/webp-uploads/hooks.php

Lines 772 to 778 in 71055b3

function webp_uploads_render_generator() {
if (
defined( 'WEBP_UPLOADS_VERSION' )
) {
echo '<meta name="generator" content="WebP Uploads ' . esc_attr( WEBP_UPLOADS_VERSION ) . '">' . "\n";
}
}

performance/plugins/embed-optimizer/hooks.php

Lines 164 to 170 in 71055b3

function embed_optimizer_render_generator() {
if (
defined( 'EMBED_OPTIMIZER_VERSION' )
) {
echo '<meta name="generator" content="Embed Optimizer ' . esc_attr( EMBED_OPTIMIZER_VERSION ) . '">' . "\n";
}
}

Aside: It seems Auto Sizes is missing a generator meta tag also.

@felixarntz
Copy link
Member Author

@westonruter

Maybe take this as an opportunity to align the other functions with this one? Namely, the constant checks seem to be a vestige of the module past.

Done in c6252cf

Aside: It seems Auto Sizes is missing a generator meta tag also.

Let's do that in a separate PR.

Another idea: Should we use the plugin slug instead of the plugin name for all those individual generator tags? That would be more in line with the main Performance Lab generator tag, and furthermore would avoid having to make a trade-off between using outdated plugin names vs the maintenance burden of having to adjust tooling every time we may change the plugin name.

Since those generator tags are still new and I assume aren't used much, it would probably be fine to change them once at this point to be the slug, and after that they'll be "stable" forever.

@westonruter
Copy link
Member

westonruter commented Mar 29, 2024 •
edited
Loading

Good idea! Let's use slugs. 🐌

@westonruter
Copy link
Member

Let's do that in a separate PR.

Why? You're already in here modifying other plugins' generator meta tags. Will reduce noise to just do it all here.

@felixarntz
Copy link
Member Author

Good idea! Let's use slugs. 🐌

I'll do that in a separate follow up PR, as it's a distinct change affecting several plugins in a way that I'd put it in the changelog.

Why? You're already in here modifying other plugins' generator meta tags. Will reduce noise to just do it all here.

Similar to the above, I prefer to do this separately as it would touch another plugin and should be mentioned in the changelog. It's quick anyway.

@mukeshpanchal27
Copy link
Member

I would also like to do the follow-up work on a separate PR.

@mukeshpanchal27 mukeshpanchal27 merged commit 9dfd069 into trunk Mar 29, 2024
27 checks passed
@mukeshpanchal27 mukeshpanchal27 deleted the add/speculative-loading-generator branch March 29, 2024 03:50
@felixarntz felixarntz mentioned this pull request Mar 29, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@westonruter westonruter westonruter approved these changes

@mukeshpanchal27 mukeshpanchal27 mukeshpanchal27 approved these changes

@thelovekesh thelovekesh thelovekesh approved these changes

@swissspidy swissspidy Awaiting requested review from swissspidy

@adamsilverstein adamsilverstein Awaiting requested review from adamsilverstein adamsilverstein is a code owner

@pbearne pbearne Awaiting requested review from pbearne pbearne is a code owner

@spacedmonkey spacedmonkey Awaiting requested review from spacedmonkey

Assignees
No one assigned
Labels
[Plugin] Speculative Loading Issues for the Speculative Loading plugin (formerly Speculation Rules) [Type] Enhancement A suggestion for improvement of an existing feature
Projects
None yet
Milestone
speculation-rules 1.1.0
Development

Successfully merging this pull request may close these issues.
4 participants
@felixarntz @westonruter @mukeshpanchal27 @thelovekesh