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

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
Copy link
Member

@mitogh mitogh 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 felixarntz changed the base branch from trunk to release/1.0.0-beta.3 March 21, 2022 18:47
@felixarntz
Copy link
Member Author

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 merged commit 803a031 into release/1.0.0-beta.3 Mar 22, 2022
@felixarntz felixarntz deleted the fix/default-timing branch March 22, 2022 15:14
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