Fix PHP cross-version compatibility issue (PHP 7) #386
Conversation
jrfnl
force-pushed
the
feature/fix-php-cross-version-compat-issue
branch
from
02db5a0
to
85ad8f4
Compare
|
@rmccue another PR we cannot merge because of the required statuses... Please lift this ! |
|
@ozh Please don't remove the required statuses. I honestly think they are a good thing. I do agree something needs to be done about the few unit tests which are prone to failing due to connection errors. Maybe those could be moved to a separate group and added as a separate "allowed to fail" build ? |
|
This project has unit tests, that's truly awesome, but since the same test suite can either fail or pass, I really think it's a real pain to prevent a responsible collaborator from clicking the "Merge pull request" button :) (typical example here: one travis batch passes, the other one fails) In the end you're right, there should be a better way to do things not to depend on a external request timeout. I know @rmccue has put hours into this so I'm not sure what could be done to make things more independent |
I'm a little aghast that nobody has come across this before and that no bug reports have come in about this (or maybe they have, but I haven't identified them as such), but the code as was was not cross-version compatible with PHP 5 vs PHP 7 due to the Uniform Variable Handling introduced in PHP 7.0. A detailed code sample demonstrating the issue can be found here: https://3v4l.org/T3MDQ The current fix maintains the pre-PHP 7.0 behaviour, which I believe, as this is an older codebase, is the intended behaviour.
jrfnl
force-pushed
the
feature/fix-php-cross-version-compat-issue
branch
from
85ad8f4
to
3e52edd
Compare
I'm a little aghast that nobody has come across this before and that no bug reports have come in about this (or maybe they have, but I haven't identified them as such), but the code as was, was not cross-version compatible with PHP 5 vs PHP 7 due to the Uniform Variable Syntax introduced in PHP 7.0.
A detailed code sample demonstrating the issue can be found here: https://3v4l.org/T3MDQ
The current fix maintains the pre-PHP 7.0 behaviour, which I believe, as this is an older codebase, is the intended behaviour.