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

Ensure default modules are loaded regardless of setting registration #248

Merged
merged 1 commit into from Mar 22, 2022

Conversation

felixarntz
Copy link
Member

@felixarntz felixarntz commented Mar 21, 2022

Summary

Fixes #247

Relevant technical choices

  • By passing the default as second parameter to get_option() in perflab_get_module_settings(), it is ensured the default value is always used, even when the setting is not registered yet.
  • Since the plugin consistently relies on the perflab_get_module_settings() function to get the setting value, this ensures the default is also taken into consideration early (e.g. when the active modules are being loaded).
  • The default is still also being registered, which is still necessary to support potential other plugins that may need it, e.g. when calling get_option() directly.
  • Tests are included to verify the expected behavior.

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] Bug An existing feature is broken Infrastructure Issues for the overall performance plugin infrastructure Needs Review Anything that requires code review labels Mar 21, 2022
@felixarntz felixarntz added this to the 1.0.0-beta.3 milestone Mar 21, 2022
@felixarntz felixarntz changed the title Ensure default modules setting value is always used regardless of setting registration Ensure default modules are loaded regardless of setting registration Mar 21, 2022
mitogh
mitogh approved these changes Mar 21, 2022
Copy link
Member

@mitogh mitogh left a comment

LGTM 🎉

@felixarntz felixarntz changed the base branch from trunk to release/1.0.0-beta.3 Mar 21, 2022
@felixarntz
Copy link
Member Author

felixarntz commented Mar 21, 2022

I've tested this PR on a fresh site and can confirm that all non-experimental modules are now loaded there right away (i.e. without updating settings under Settings > Performance), when before they weren't:

  • WebP images are being generated.
  • The Site Health check for WebP shows up.
  • The Site Health check for a persistent object cache shows up.

@felixarntz felixarntz requested a review from ThierryA Mar 22, 2022
@felixarntz felixarntz merged commit 803a031 into release/1.0.0-beta.3 Mar 22, 2022
20 checks passed
@felixarntz felixarntz deleted the fix/default-timing branch Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Issues for the overall performance plugin infrastructure Needs Review Anything that requires code review [Type] Bug An existing feature is broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default value on settings is not set on new installations
6 participants