Skip to content
This repository has been archived by the owner on Jun 3, 2019. It is now read-only.

Fix persistent errors in plugins #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Sep 28, 2016

The new version of VVV will be running PHP 7.

The Log Deprecated Notices plugin doesn't really have an alternative, but is not actively maintained nor has it got a GH shadow with more up to date code.
This plugin has a PHP7 compatibility issue which this adjustment fixes.

The WordPress Importer is needed to import the theme test data, but also has PHP7 compatibility issues. HumanMade is working on a refactor, but replacing the current plugin with the refactored one broke the importing of the unit test data, so for now, fixing the PHP 7 issue in the WordPress Importer plugin seemed the simpler solution.
Ref: https://github.com/humanmade/WordPress-Importer

The fixes are made by editing the actual plugin files. This is done right after install, but also at the end of the script so as to fix already installed versions of the plugins.

This gets rid of the following errors which will show in the VVV provision logs and/or PHP error logs:

# Log Deprecated Notices
Deprecated: Methods with the same name as their class will not be constructors in a future version of PHP; Deprecated_Log has a deprecated constructor in /srv/www/wordpress-themereview-vvv/htdocs/wp-content/plugins/log-deprecated-notices/log-deprecated-notices.php on line 25

# WordPress Importer
Notice: Array to string conversion in /srv/www/wordpress-themereview-vvv/htdocs/wp-content/plugins/wordpress-importer/wordpress-importer.php on line 884
Notice: Undefined variable: _menu_item_type in /srv/www/wordpress-themereview-vvv/htdocs/wp-content/plugins/wordpress-importer/wordpress-importer.php on line 886
Notice: Undefined variable: _menu_item_type in /srv/www/wordpress-themereview-vvv/htdocs/wp-content/plugins/wordpress-importer/wordpress-importer.php on line 888
Notice: Undefined variable: _menu_item_type in /srv/www/wordpress-themereview-vvv/htdocs/wp-content/plugins/wordpress-importer/wordpress-importer.php on line 890

The new version of VVV will be running PHP 7.

The Log Deprecated Notices plugin doesn't really have an alternative, but is not actively maintained nor has it got a GH shadow with more up to date code.
This plugin has a PHP7 compatibility issue which this adjustment fixes.

The WordPress Importer is needed to import the theme test data, but also has PHP7 compatibility issues. HumanMade is working on a refactor, but replacing the current plugin with the refactored one broke the importing of the unit test data, so for now, fixing the PHP 7 issue in the WordPress Importer plugin seemed the simpler solution.
Ref: https://github.com/humanmade/WordPress-Importer

The fixes are made by editing the actual plugin files. This is done right after install, but also at the end of the script so as to fix already installed versions of the plugins.

This gets rids of the following errors which will show in the VVV provision logs and/or PHP error logs:
```
# Log Deprecated Notices
Deprecated: Methods with the same name as their class will not be constructors in a future version of PHP; Deprecated_Log has a deprecated constructor in /srv/www/wordpress-themereview-vvv/htdocs/wp-content/plugins/log-deprecated-notices/log-deprecated-notices.php on line 25

# WordPress Importer
Notice: Array to string conversion in /srv/www/wordpress-themereview-vvv/htdocs/wp-content/plugins/wordpress-importer/wordpress-importer.php on line 884
Notice: Undefined variable: _menu_item_type in /srv/www/wordpress-themereview-vvv/htdocs/wp-content/plugins/wordpress-importer/wordpress-importer.php on line 886
Notice: Undefined variable: _menu_item_type in /srv/www/wordpress-themereview-vvv/htdocs/wp-content/plugins/wordpress-importer/wordpress-importer.php on line 888
Notice: Undefined variable: _menu_item_type in /srv/www/wordpress-themereview-vvv/htdocs/wp-content/plugins/wordpress-importer/wordpress-importer.php on line 890
```
@aubreypwd
Copy link
Owner

I left some comments, but I'm not sure I like the idea of modifying files directly. Maybe you could write a .sh file someone can download to fix it? Maybe we could reference it?

@jrfnl
Copy link
Contributor Author

jrfnl commented Oct 13, 2016

The problem is that with the new version of VVV the import of the theme data will partially fail because of these errors, making the themereview auto-site a lot less useful.
Most theme reviewers will not be comfortable editing a VVV script to add a call to another .sh file into it, so that basically would mean that this repo is deprecated and should not be used anymore. will no longer be useable for most theme reviewers with the latest VVV version.

@jrfnl
Copy link
Contributor Author

jrfnl commented Oct 13, 2016

P.S.: where did you leave the comments ? I can't seem to find them.

@aubreypwd aubreypwd modified the milestone: 1.2.2 Oct 13, 2016
@aubreypwd
Copy link
Owner

I left them in the diff's, and thanks for helping out! But, I just think this is a wordpress-importer problem and we should wait for them to fix it there?

@jrfnl
Copy link
Contributor Author

jrfnl commented Oct 13, 2016

It's weird, I cannot see your comments in the diff.

Regarding WP Importer, it's an Automattic plugin which is barely maintained anymore. There have only been three very minimal patch releases in the last five years.

@jrfnl
Copy link
Contributor Author

jrfnl commented Oct 13, 2016

Oh ... you may need to click "Finish review" for your comments to become visible. Could that be it ?

@aubreypwd aubreypwd self-assigned this Oct 13, 2016
@@ -51,7 +51,13 @@ PHP
# **

printf 'Installing plugins...\n'
wp plugin install wordpress-importer --activate --allow-root
wp plugin install wordpress-importer --allow-root
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the reason we're not activating this is because it will throw an error?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait I see it below being activated, so this actually modifies the code, not sure I'm good with that happening on init.

# Fix PHP4 constructor in the Log Deprecated Notices plugin.
echo "Removing errors from the Log Deprecated Notices plugin."
if [[ -f /srv/www/wordpress-themereview-vvv/htdocs/wp-content/plugins/log-deprecated-notices/log-deprecated-notices.php ]]; then
sed -i -e 's/function Deprecated_Log() {/function __construct() {/g' "/srv/www/wordpress-themereview-vvv/htdocs/wp-content/plugins/log-deprecated-notices/log-deprecated-notices.php"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, again not sure about modifying actual code; I think the developer would do this manually. Maybe we can do a separate .sh file to do this? Maybe?

@aubreypwd aubreypwd removed their assignment Sep 29, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
2 participants