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

felixarntz
Copy link
Member

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 Documentation to be added or enhanced [Focus] Site Health LEGACY LEGACY – Issues related to the Site Health focus area Needs Review Anything that requires code review [Module] Audit Enqueued Assets Issues for the Audit Enqueued Assets Health Check module labels Mar 3, 2022
@felixarntz felixarntz added this to the 1.0.0-beta.1 milestone Mar 3, 2022
Copy link
Member

@ThierryA ThierryA left a comment

Choose a reason for hiding this comment

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

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

@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

@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

Choose a reason for hiding this comment

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

LGTM 👍

@felixarntz
Copy link
Member Author

@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
@tillkruss tillkruss deleted the update/sh-audit-enqueued-assets-experimental branch March 7, 2022 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Site Health LEGACY LEGACY – Issues related to the Site Health focus area [Module] Audit Enqueued Assets Issues for the Audit Enqueued Assets Health Check module Needs Review Anything that requires code review [Type] Documentation Documentation to be added or enhanced
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants