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

PHP 7.4 compatibility fix / implode argument order #88

Merged
merged 1 commit into from
Oct 2, 2019

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Jul 31, 2019

implode() takes two parameters, $glue and $pieces.
For historical reasons, implode() accepted these parameters in either order, though it was recommended to use the documented argument order of implode( $glue, $pieces ).

PHP 7.4 deprecates the tolerance for passing the parameters for implode() in reverse order.
PHP 8.0 is expected to remove the tolerance for this completely.

Refs:

Verifying this fix

I've turned on a build against master: https://travis-ci.org/Yoast/whip/jobs/565998239
As you can see, the Travis builds on PHP 7.4 are failing. This is caused by the above issue.

When you look at the Travis output for this PR - https://travis-ci.org/Yoast/whip/jobs/565996627 -, you will see that the PHP 7.4 build is passing again.

`implode()` takes two parameters, `$glue` and `$pieces`.
For historical reasons, `implode()` accepted these parameters in either order, though it was recommended to use the documented argument order of `implode( $glue, $pieces )`.

PHP 7.4 deprecates the tolerance for passing the parameters for `implode()` in reverse order.
PHP 8.0 is expected to remove the tolerance for this completely.

Refs:
* https://wiki.php.net/rfc/deprecations_php_7_4#implode_parameter_order_mix
* https://php.net/manual/en/function.implode.php
@jrfnl jrfnl added this to the Next release milestone Jul 31, 2019
@IreneStr IreneStr merged commit e2a611c into master Oct 2, 2019
@IreneStr IreneStr deleted the JRF/php-7.4/fix-implode-reverse-order branch October 2, 2019 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants