-
Notifications
You must be signed in to change notification settings - Fork 98
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
Remove modules infrastructure #1030
Comments
@mukeshpanchal27 @joemcgill @thelovekesh Let's work on this once #1029 is completed. This should be a big PR, but almost only red lines, which is always a good thing :) |
@felixarntz Do we needs to show |
@mukeshpanchal27 I don't think so. They aren't modules nor plugins, and they're just active all the time that Performance Lab is active. So having them in the generator tag wouldn't serve any purpose. |
This list looks pretty comprehensive. One question:
Do we need a new generated file for these translations or is this a legacy file that can be deleted entirely? |
@joemcgill The file can be deleted, since the translations were only needed there because the source strings came from the module headers, which by default wouldn't have been picked up by the .org translation engine. But we don't have modules anymore, so the file is no longer relevant. Any references to the plugin names would now happen in regular translation strings anyway. |
Thanks. Considering that in the current version, users have the option to enable or disable the module, if some sites disable those site health checks, then in the next version (3.0), if they are enabled by default, how should we manage that? Do we need to add an option to disable them again? |
I don't think so. They don't change the functionality of the site, I think it's okay to just permanently have them active. Similar to how Server-Timing is permanently active when using Performance Lab. I don't think we need to put any messaging in place for that. |
@mukeshpanchal27 This is now unblocked following the merge of #1042. |
@mukeshpanchal27 Are you going to pick it up? |
Yes, I've already started working on it. |
@mukeshpanchal27 After you rightfully raised it as part of #1064, I have updated the documentation in https://make.wordpress.org/performance/handbook/performance-lab/releasing-the-plugin/ to reflect the changes made in #1060 and #1064. This simplifies not only creating PRs, but also the release process. Next steps:
|
Following #1029, there are effectively no longer any modules as part of Performance Lab. This should remain that way. As such, all modules infrastructure should be entirely removed.
Let's focus on the minimum effort needed to clean up in this issue. Further enhancements to user experience should be considered in separate issues.
Here is a non comprehensive list of what needs to be removed:
perflab_is_standalone_plugin_loaded()
PHP function (as it relates to modules).bin
that deal with modules exclusively.module-i18n.php
,default-enabled-modules.php
, and their usage and surrounding infrastructure.Additional changes to be made:
plugins.json
file should no longer support a"modules"
entry. Tooling (e.g. scripts and GitHub workflows) should be updated to no longer consider that"modules"
entry. For example, thedeploy-standalone-plugins.yml
workflow should be updated to only work with the actual plugins in the/plugins
folder and no longer reference modules, which means for example scripts likeget-plugin-dir
can be removed as it will now always be the/plugins
directory.$source
parameter of theperflab_get_standalone_plugin_version_constants()
function should be deprecated (or removed) and effectively always be for plugins.Performance Lab %1$s; plugins: %2$s
going forward.readme.txt
should be updated to no longer list the available modules.readme
JS script should be adjusted to no longer try to populate the list of available modules intoreadme.txt
(the changelog functionality should be kept though).These lists may not be complete, and further things to remove/update may come up as a PR is being worked on.
The text was updated successfully, but these errors were encountered: