[WIP] enable php7 travis #9858
[WIP] enable php7 travis #9858
Conversation
|
Starting with 66 failures in |
|
@tsteur @mattab since i started to work on fixing the issues of the sorting between PHP5 / 7 i got down from 66 failures to 32 failures (1 failure in submodule is also fixed, so 31). I only changed this sorting method and fixed the expected results Now it sorts by the count and then the label, instead of only the count. _My question_ |
| } | ||
|
|
||
| //equal count -> so sort by the label | ||
| return strcmp($a['label'], $b['label']); |
There was a problem hiding this comment.
This should work as there should never be the same label twice. In other case we should probably fall back to the position in the array (array index) or so.
It will make things slower, but I think there is no way around as we have to run tests on PHP 7 as well. Maybe it'll be faster in some other cases where we can also use the index position maybe somehow. I reckon the only case where performance re sorting matters most is here: https://github.com/piwik/piwik/blob/39f6964dbfd3b66520dd04728b64cdc856a532c3/core/DataTable/Filter/Sort.php#L250-L289 It shouldn't add too much overhead though.
Yes, it's awesome you're working on this. I'm looking forward to merge this once tests are green. |
mattab
added
c: Tests & QA
|
Hi @ThaDafinser - Thanks for the PR. Would you be interested to continue the work on this so we can merge it and run our CI job |
|
Since i started studiing, i'm afraid i can't complete it the next weeks. The PR would also need a complete rebase - i think best is to open a complete new one and take some parts out of it. |
|
Thanks for the update @ThaDafinser - all the best for your studies! Created this issue: #10586 |
No description provided.