PHP 7.4 compatibility fix / implode argument order#346
Conversation
`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 is slated to deprecate 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
|
The Travis failure seems unrelated to this PR. Looks like the DevOps site of this repo needs a refresh. |
|
Anything I can do to move this patch forward ? WordPress 5.3 should include this patch, so it would be great if it could be merged and a new version of Requests tagged before WP 5.3 goes into RC.... |
|
@rmccue can we consider removing the mandatory tests? Some patches are fairly trivial but tests always fail anyway ... |
Wouldn't fixing them be more effective ? |
|
See #351 for the tests. |
|
I'll happily rebase this PR once #351 is merged to show the tests passing. Just ping me. |
|
#351 is merged now, so should be good to merge in here. (If you could, please merge rather than rebase, thanks!) |
I will if you insist, but I honestly don't understand why you'd want that ? |
|
I prefer preserving the history. :) |
|
@rmccue But there is no history to preserve... all a normal rebase does is re-attach my branch at the head of the current |
Codecov Report
@@ Coverage Diff @@
## master #346 +/- ##
=========================================
Coverage 92.11% 92.11%
Complexity 760 760
=========================================
Files 21 21
Lines 1762 1762
=========================================
Hits 1623 1623
Misses 139 139
Continue to review full report at Codecov.
|
rmccue
left a comment
rmccue
left a comment
There was a problem hiding this comment.
Patch looks good; I've restarted the failing tests as they appear to be transient errors (again; not sure why these are more frequent these days...)
|
@rmccue Could you (selectively) do that again ? Builds failing again on unrelated issues. |
|
@rmccue Two more to restart (again).... |
|
Yep, restarting them in smaller blocks, as I think the issue is with the tests server not supporting enough concurrency. All looking good so far... |
|
Phew, finally, thanks so much for this! We should add 7.3 and 7.4 to the testing matrix as well as non-failing. |
@rmccue I've just prepared a PR for that and am waiting for a passing build to pull it ;-) |
4 participants
implode()takes two parameters,$glueand$pieces.For historical reasons,
implode()accepted these parameters in either order, though it was recommended to use the documented argument order ofimplode( $glue, $pieces ).PHP 7.4 is slated to deprecate the tolerance for passing the parameters for
implode()in reverse order.PHP 8.0 is expected to remove the tolerance for this completely.
Refs: