Check for required PHP extensions upon plugin initialization #4930
Merged
swissspidy merged 37 commits intomainfrom Nov 2, 2020
Merged
Check for required PHP extensions upon plugin initialization #4930swissspidy merged 37 commits intomainfrom
swissspidy merged 37 commits intomainfrom
Conversation
Contributor
|
Size Change: 0 B Total Size: 1.4 MB ℹ️ View Unchanged
|
Codecov Report
@@ Coverage Diff @@
## main #4930 +/- ##
==========================================
- Coverage 76.15% 75.81% -0.34%
==========================================
Files 920 921 +1
Lines 16294 16381 +87
==========================================
+ Hits 12409 12420 +11
- Misses 3885 3961 +76
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Contributor
Author
|
CC @westonruter and @schlessera as this PR may interest them. |
westonruter
reviewed
Oct 21, 2020
swissspidy
reviewed
Oct 26, 2020
spacedmonkey
commented
Oct 27, 2020
spacedmonkey
commented
Oct 27, 2020
spacedmonkey
commented
Oct 27, 2020
spacedmonkey
commented
Oct 27, 2020
spacedmonkey
commented
Oct 27, 2020
swissspidy
reviewed
Oct 27, 2020
includes/namespace.php
Outdated
| add_action( 'admin_notices', __NAMESPACE__ . '\_print_missing_build_admin_notice' ); | ||
|
|
||
| if ( ( defined( 'WP_CLI' ) && WP_CLI ) || 'true' === getenv( 'CI' ) || 'cli' === PHP_SAPI ) { | ||
| // In CLI context, existence of the JS files is not required. |
Collaborator
There was a problem hiding this comment.
Let's ditch this.
While existence of the JS files is not required in CLI context, this should be surfaced to the user nonetheless, because it still means the plugin is incomplete and not working as expected.
That means we can use the same checks in CLI context and regular context.
That means we can just use one run_checks() method in both places.
Contributor
Author
There was a problem hiding this comment.
@swissspidy I remember why we did this in the first place. Without the javascript files the unit tests fail. Building javascript will take time and massively slow down the phpunit tests runs. I have a workaround here a43bae9
Contributor
Author
There was a problem hiding this comment.
swissspidy
reviewed
Oct 28, 2020
swissspidy
reviewed
Oct 28, 2020
swissspidy
reviewed
Oct 28, 2020
swissspidy
reviewed
Oct 28, 2020
swissspidy
reviewed
Oct 28, 2020
Closed
swissspidy
reviewed
Oct 30, 2020
swissspidy
reviewed
Oct 30, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Create a new
Compatibilityclass and move repeated logic like checks for PHP and WordPress versions to new class. Each method on the class implements a check and can be called from different contexts. The class has some new checks, mostly copied from the AMP plugin.Relevant Technical Choices
Implementing these checks as a class means if the plugin isn't built then the class file has to be loaded manually. There is no way around this but it does work pretty well in my testing.
Lots of the checks are set as class properties. There has two benefits.
Standard gettings and setters have been used to populate values into class properties. This makes the signature of the contractor a little cleaner.
This list of extensions, functions and classes was generated by doing a lot of research into the code. It is different from the amp list. Specially the json extension check.
To-do
User-facing changes
One implement, is that if multiple check fail, this will be displayed to the user, using bullet points. See this example.
Testing Instructions
Installed plugin on site that doesn't have stories min require, like PHP 5.5 or WordPress 5.2 and active plugin. See warning message.
Fixes #4791