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

Stream fails gracefully on PHP versions lower than 5.3 #781

Merged
merged 7 commits into from Oct 16, 2015

Conversation

Projects
None yet
3 participants
@greguly
Copy link
Contributor

commented Oct 8, 2015

Fixes #778

@lukecarbis

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2015

Thanks so much for this PR @greguly. Could you kindly revert c2703b9, and revert the plugin version to 3.0.2? I'll do that myself in the develop branch when I'm ready to release it.

Also - rather than including a new file, have you considered this approach? I feel it's a lot cleaner, but I've not tested if it works:

if ( ! version_compare( PHP_VERSION, '5.3', '>=' ) ) {
    load_plugin_textdomain( 'stream', false, dirname( plugin_basename( __FILE__ ) ) . '/languages/' );
    add_action( 'shutdown', 'wp_stream_fail_php_version' );
 } else {
    require __DIR__ . '/classes/class-plugin.php';
    $plugin_class_name = 'WP_Stream\\Plugin';
    if ( class_exists( $plugin_class_name ) ) {
        $GLOBALS['wp_stream'] = new $plugin_class_name();
    }
 }

Finally, what's your rationale for removing wp_stream_get_instance()? I know a lot of Stream extensions use this as a cleaner / future-proof way of retrieving the Stream instance.

@rob

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2015

@lukecarbis, I can confirm your code snippet works with PHP 5.2. I like your approach better instead of including another file. (I don't think you need to escape the backslash in WP_Stream\\Plugin though in this case.)

@greguly

This comment has been minimized.

Copy link
Contributor Author

commented Oct 9, 2015

@rob got it correctly, there is no need to escape the backslash :-)

Of course will revert c2703b9, will also fix the backslash and use your solution.

My rationale for including a new file was to isolate all 5.3 code in one place, if you pay attention function wp_stream_get_instance() was moved there too.

greguly added some commits Oct 9, 2015

@greguly

This comment has been minimized.

Copy link
Contributor Author

commented Oct 9, 2015

All set :-)

stream.php Outdated
@@ -3,7 +3,7 @@
* Plugin Name: Stream
* Plugin URI: https://wp-stream.com/
* Description: Stream tracks logged-in user activity so you can monitor every change made on your WordPress site in beautifully organized detail. All activity is organized by context, action and IP address for easy filtering. Developers can extend Stream with custom connectors to log any kind of action.
* Version: 3.0.2
* Version: 3.0.3

This comment has been minimized.

Copy link
@lukecarbis

lukecarbis Oct 9, 2015

Contributor

@greguly Can you please just switch this line back?

lukecarbis pushed a commit that referenced this pull request Oct 16, 2015

Luke Carbis
Merge pull request #781 from greguly/develop
Stream fails gracefully on PHP versions lower than 5.3

@lukecarbis lukecarbis merged commit 2f7035f into xwp:develop Oct 16, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@lukecarbis

This comment has been minimized.

Copy link
Contributor

commented Oct 16, 2015

💥 Thanks @greguly

@greguly

This comment has been minimized.

Copy link
Contributor Author

commented Oct 16, 2015

You are welcome, it was my pleasure to help. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.