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

[4.8] Avoid using function each() #2773

Closed
wants to merge 6 commits into from

Conversation

mlocati
Copy link

@mlocati mlocati commented Sep 12, 2017

Backport of 48930fc by @eclipxe13 to the 4.8 series

@SpacePossum
Copy link
Contributor

cherry-pick of that commit didn't work?

@eclipxe13
Copy link
Contributor

I think other commit is missing, the full PR is about 4 commits, 2 of tests and 2 of code.
https://github.com/sebastianbergmann/phpunit/pull/2536/commits

@SpacePossum
Copy link
Contributor

maybe try picking the 4? having the tests would be nice in this branch as well I guess :)

@mlocati
Copy link
Author

mlocati commented Sep 12, 2017

@eclipxe13 You are right!

@SpacePossum I manually rewrote the 4th commit, cherry picked the two commits for tests, and fixed the tests for older PHP and PHPUnit


public function testItHandlesLongParametesWithValues()
{
$command = 'command parameter-0 --exec parameter-1 --conf config.xml --optn parameter-2 --optn=content-of-o parameter-n';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line exceeds 120 characters; contains 129 characters

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks bot 😉

@mlocati
Copy link
Author

mlocati commented Sep 12, 2017

cherry-pick of that commit didn't work?

I had a merge conflict, so I preferred to rewrite it by hand

@Remo
Copy link

Remo commented Sep 21, 2017

Yes please, that would fix the build for the legacy version of concrete5 https://github.com/concrete5/concrete5-legacy/

@sebastianbergmann
Copy link
Owner

sebastianbergmann commented Sep 21, 2017

PHPUnit 4.8 is no longer supported (and never supported PHP 7).

@mlocati
Copy link
Author

mlocati commented Sep 21, 2017

@sebastianbergmann We need to test a software that supports PHP from version 5.3 to 7.2 (dropping end-of-life PHP versions is not an option for us). Do you have any suggestion about how could we do that?

@hboomsma
Copy link

You could always fork your preferred version of PHPUnit and maintain it yourself since no new commits will be put in the upstream one.

@mlocati
Copy link
Author

mlocati commented Sep 21, 2017

@hboomsma thanks for the feedback. I already know we can fork phpunit, but I was wondering if @sebastianbergmann had a better approach, since I think we are not the only one with this need...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants