close
The Wayback Machine - https://web.archive.org/web/20220410225820/https://github.com/WordPress/performance/pull/205
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

Mark Site Health Audit Enqueued Assets module as experimental for now. #205

Conversation

Copy link
Member

@felixarntz felixarntz commented Mar 3, 2022

Summary

Follow-up for #25

Relevant technical choices

  • Mark module as experimental based on latest comments.

Checklist

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

@felixarntz felixarntz added [Type] Documentation [Focus] Site Health Needs Review [Module] Audit Enqueued Assets labels Mar 3, 2022
@felixarntz felixarntz added this to the 1.0.0-beta.1 milestone Mar 3, 2022
@felixarntz felixarntz requested a review from audrasjb as a code owner Mar 3, 2022
@felixarntz felixarntz requested review from JustinyAhin and ThierryA Mar 3, 2022
Copy link
Member

@ThierryA ThierryA left a comment

LGTM

Making this experimental makes sense since:

  1. it still has open follow up issue
  2. some concern about the threshold was raised lately
  3. it is much easier to start as experimental and then upgrade to stable than the other way around

@felixarntz
Copy link
Member Author

@felixarntz felixarntz commented Mar 3, 2022

@manuelRod Would be great to get your feedback on the issue / this PR as well since you've done most of the work on the module. To clarify, it's not that there is a problem with the module, it's more about that it doesn't feel as polished yet, hence marking as "experimental" for this first release - as it sees more refinement over time, we could then mark it as non-experimental.

@manuelRod
Copy link
Contributor

@manuelRod manuelRod commented Mar 4, 2022

@felixarntz I left some feedback on the previous issue 25.

To sum up:
Said that I'm not totally sure if marking this as experimental ( I'm not against either ) would make a difference. To me, experimental sounds more like a change that may break your site or cause unexpected behaviors, this is just an inoffensive warning in the site health screen. And at this stage, this whole plugin is experimental itself, so people using it are kinda aware of it ( or at least, that's how I see it ).

To sum up, I see "experimental" as a different thing, but if we all agree, I'm in too.

Copy link

@audrasjb audrasjb left a comment

LGTM 👍

@felixarntz
Copy link
Member Author

@felixarntz felixarntz commented Mar 4, 2022

@manuelRod

experimental sounds more like a change that may break your site or cause unexpected behaviors

Indeed this wouldn't break anyone's site, but IMO the main point for marking this as experimental is that it's still in an earlier stage of development compared to the other modules. There are still foundational iterations happening, e.g. we haven't fully defined what the thresholds should be, and the approach to gathering the assets is known to be not yet reliable for certain environments. We also need to think about how to make this warning more actionable, e.g. at a minimum tell users which plugins or themes are responsible for enqueuing the majority of assets etc.

As we work towards these enhancements and define the intended scope of the module further, eventually we could then remove the experimental label from it.

@felixarntz felixarntz merged commit e78f9c6 into release/1.0.0-beta.1 Mar 4, 2022
17 checks passed
@tillkruss tillkruss deleted the update/sh-audit-enqueued-assets-experimental branch Mar 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Site Health [Module] Audit Enqueued Assets Needs Review [Type] Documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants