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

Remove modules infrastructure #1030

Closed
Tracked by #656
felixarntz opened this issue Mar 6, 2024 · 11 comments · Fixed by #1060
Closed
Tracked by #656

Remove modules infrastructure #1030

felixarntz opened this issue Mar 6, 2024 · 11 comments · Fixed by #1060
Assignees
Labels
Creating standalone plugins Infrastructure Issues for the overall performance plugin infrastructure Performance Lab Plugin Issue relates to work in the Performance Lab Plugin only [Type] Enhancement A suggestion for improvement of an existing feature

Comments

@felixarntz
Copy link
Member

felixarntz commented Mar 6, 2024

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:

  • All UI related to toggling modules (i.e. the checkboxes).
  • All UI related to the module migration.
  • All PHP functions related to module settings.
  • All PHP functions related to loading modules, module activation/deactivation etc.
  • All PHP functions related to migrating from modules to their standalone plugins.
  • The perflab_is_standalone_plugin_loaded() PHP function (as it relates to modules).
  • All unit test coverage related to module infrastructure.
  • All CLI scripts in bin that deal with modules exclusively.
  • All GitHub workflows that deal with modules exclusively (such as building and testing modules, publishing modules, ...).
  • Utility files, like module-i18n.php, default-enabled-modules.php, and their usage and surrounding infrastructure.
  • Any reference to modules, really :)

Additional changes to be made:

  • The 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, the deploy-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 like get-plugin-dir can be removed as it will now always be the /plugins directory.
  • The $source parameter of the perflab_get_standalone_plugin_version_constants() function should be deprecated (or removed) and effectively always be for plugins.
  • The generator tag should be adjusted to not include any information about modules. In other words, it output should look like Performance Lab %1$s; plugins: %2$s going forward.
  • The copy of the default admin pointer should be adjusted to no longer say "performance features included in the plugin", but just "performance features".
  • The main readme.txt should be updated to no longer list the available modules.
  • The readme JS script should be adjusted to no longer try to populate the list of available modules into readme.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.

@felixarntz felixarntz added [Type] Enhancement A suggestion for improvement of an existing feature Infrastructure Issues for the overall performance plugin infrastructure Creating standalone plugins Performance Lab Plugin Issue relates to work in the Performance Lab Plugin only labels Mar 6, 2024
@felixarntz felixarntz added this to the PL Plugin 3.0.0 milestone Mar 6, 2024
@felixarntz
Copy link
Member Author

@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 :)

@mukeshpanchal27
Copy link
Member

The generator tag should be adjusted to not include any information about modules. In other words, it output should look like Performance Lab %1$s; plugins: %2$s going forward.

@felixarntz Do we needs to show site-health directory plugins in generator tag?

@felixarntz
Copy link
Member Author

@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.

@joemcgill
Copy link
Member

This list looks pretty comprehensive. One question:

Utility files, like module-i18n.php...

Do we need a new generated file for these translations or is this a legacy file that can be deleted entirely?

@felixarntz
Copy link
Member Author

@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.

@mukeshpanchal27
Copy link
Member

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.

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?

@felixarntz
Copy link
Member Author

felixarntz commented Mar 7, 2024

@mukeshpanchal27

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 mukeshpanchal27 self-assigned this Mar 11, 2024
@felixarntz
Copy link
Member Author

@mukeshpanchal27 This is now unblocked following the merge of #1042.

@thelovekesh
Copy link
Member

@mukeshpanchal27 Are you going to pick it up?

@mukeshpanchal27
Copy link
Member

@mukeshpanchal27 Are you going to pick it up?

Yes, I've already started working on it.

@felixarntz
Copy link
Member Author

felixarntz commented Mar 21, 2024

@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:

cc @westonruter @thelovekesh @joemcgill

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Creating standalone plugins Infrastructure Issues for the overall performance plugin infrastructure Performance Lab Plugin Issue relates to work in the Performance Lab Plugin only [Type] Enhancement A suggestion for improvement of an existing feature
Projects
Development

Successfully merging a pull request may close this issue.

4 participants