PHP 7.4 compatibility fix / implode argument order

[jrfnl]

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

[jrfnl]

The Travis failure seems unrelated to this PR. Looks like the DevOps site of this repo needs a refresh.

[jrfnl]

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....

[ozh]

@rmccue can we consider removing the mandatory tests? Some patches are fairly trivial but tests always fail anyway ...

[jrfnl]

can we consider removing the mandatory tests? Some patches are fairly trivial but tests always fail anyway ...

Wouldn't fixing them be more effective ?

[rmccue]

See #351 for the tests.

[jrfnl]

I'll happily rebase this PR once #351 is merged to show the tests passing. Just ping me.

[rmccue]

#351 is merged now, so should be good to merge in here.

(If you could, please merge rather than rebase, thanks!)

[jrfnl]

(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 ?
https://blog.developer.atlassian.com/stop-foxtrots-now/

[rmccue]

I prefer preserving the history. :)

[jrfnl]

@rmccue But there is no history to preserve... all a normal rebase does is re-attach my branch at the head of the current master, the commit stays the same (including commit date etc).

[codecov-io]

Codecov Report

Merging #346 into master will not change coverage.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master     #346   +/-   ##
=========================================
  Coverage     92.11%   92.11%           
  Complexity      760      760           
=========================================
  Files            21       21           
  Lines          1762     1762           
=========================================
  Hits           1623     1623           
  Misses          139      139

Impacted Files	Coverage Δ	Complexity Δ
library/Requests/Transport/fsockopen.php	94.18% <100%> (ø)	68 <0> (ø)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e4fe0eb...2aced35. Read the comment docs.

[rmccue]

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...)

[jrfnl]

@rmccue Could you (selectively) do that again ? Builds failing again on unrelated issues.

[jrfnl]

@rmccue Two more to restart (again)....

[rmccue]

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...

[rmccue]

Phew, finally, thanks so much for this! We should add 7.3 and 7.4 to the testing matrix as well as non-failing.

[jrfnl]

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 ;-)