-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
61103 add default autoload core #6465
Conversation
Updated values of 'autoload' option from 'yes' or 'no' to 'on' or 'off' respectively, across several PHP files. This change was implemented in the functions handling options and lock settings, enhancing clarity and consistency of the code.
The commit modifies the 'can_compress_scripts' options in the ajax-actions.php file. The update_site_option and update_option functions now have a third argument which controls whether the setting is on or off. This enhances flexibility and control over the script compression feature.
The commit adjusts the parameters for 'update_option' functions in 'featured-content.php' and 'ajax-actions.php'. It changes the 'autoload' parameter where necessary, updating the values for "featured-content" and "can_compress_scripts" options. It's done to ensure the correct use of the autoloading feature in WordPress functionality.
This commit adds a third argument to the update_option() function calls across a large number of scripts. This argument toggles whether the option should be loaded from cache (on) or not (off). The changes made help to better manage data caching in WordPress and potentially improve performance by avoiding unnecessary database queries.
This commit adds a third argument to the update_option() function calls across a large number of scripts. This argument toggles whether the option should be loaded from cache (on) or not (off). The changes made help to better manage data caching in WordPress and potentially improve performance by avoiding unnecessary database queries.
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @paul, @[email protected]. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. Core Committers: Use this line as a base for the props when committing in SVN:
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
Co-authored-by: Mukesh Panchal <[email protected]>
Updated several calls to the update_option function throughout the codebase to use boolean values for autoload parameter instead of 'off'/'on' strings. This change improves the consistency and readability of the related database entries.
I have replaced the on/off with booleans As I have gone through these I have set any option that looks like it is wp-admin only to not autoload this should help the front-end load a little bit |
Updated several calls to the update_option function throughout the codebase to use boolean values for autoload parameter instead of 'off'/'on' strings. This change improves the consistency and readability of the related database entries.
Updated the WordPress database version in `version.php` and performed changes in `upgrade.php` to reflect this. Introduced a new function `upgrade_670()` to execute changes made in WordPress version 6.7.0. Additionally, adjusted the indentation in numerous lines for code readability and consistency.
A new function named 'valid_transient' has been introduced to verify if a transient and its value are valid, also by checking if they have not expired. Sites with transient data can use the 'valid_site_transient' function to perform similar checks. Along with these functions, PHPUnit test cases have been added to ensure these functions perform as expected.
This reverts commit 014cf5d.
A new function, wp_prime_wp_admin_option_caches, has been created to prime specific network options for the current network into the cache using a single database query. Additionally, the action 'admin_init' has been added to autoload these admin options. This should lead to improved performance by reducing the number of database queries.
… into 61103-add-default-autoload-core
Co-authored-by: Mukesh Panchal <[email protected]>
Co-authored-by: Mukesh Panchal <[email protected]>
Co-authored-by: Mukesh Panchal <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @pbearne for the updates. Left some feedbacks.
Could you fix your bot that keeps mentioning me? I'm getting dozens of notifications a month from this project. https://github.com/WordPress/wordpress-develop/pulls?q=sort%3Aupdated-desc+%22%40paul%22 |
Co-authored-by: Mukesh Panchal <[email protected]>
Co-authored-by: Mukesh Panchal <[email protected]>
@pbearne Are you able to look at the details in your git config so the bot stops pinging the wrong Paul. It's currently set to If you add |
updated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pbearne I did a thorough review of all the options you're (potentially) modifying here.
My most important feedback is: We should only add true
or false
to update_option()
for options that aren't already added to the database during WordPress installation. Any such options will always already be set when calling update_option()
.
It is only mandatory to pass an $autoload
value to add_option()
because here we're inserting a new option. On update_option()
, $autoload
however is only needed if it may pass through to add_option()
or if we explicitly want to change the autoload value from what it was before. Otherwise it should be null
.
Also: We shouldn't pass true
on update_option()
calls that are for any arbitrary options. For that, the default null
is intended, which is to leave it up to WordPress core to make the decision.
I think most of the options that are not part of the WordPress installation and that you changed to no longer autoload, it makes sense because they're only used in 1-2 specific admin sections. There are a few though I left comments on too where I think this is too risky and should be separately considered.
Removed the deprecated third parameter from multiple update_option calls across various files for consistency and to avoid warnings. This change ensures that all update_option calls follow the current best practices in WordPress development.
Changed the value of 'alloptions_count' from 100 to 18 to reflect the accurate count after the expended 20. This ensures the site health test results are precise and consistent with the current state.
Changed assertions in the wpGetArchives test to reflect the correct number of queries being executed. This ensures proper validation of queries after recent changes in the query logic.
Refactored various instances of update_option function calls to exclude the 'autoload' parameter for improved code simplicity and performance. Also removed the now redundant wp_prime_wp_admin_option_caches function and its related filter.
… into 61103-add-default-autoload-core
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pbearne There are a few more instances from my previous review that still need to be reverted, but it's getting close.
I can make some of these updates inline.
Committed in https://core.trac.wordpress.org/changeset/58975 |
Trac ticket: https://core.trac.wordpress.org/ticket/61103