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

Fixed PHPUnit warnings in PHP >= 5.6 #167

Closed
wants to merge 2 commits into from

Conversation

Bilge
Copy link
Contributor

@Bilge Bilge commented Jul 8, 2016

This builds on #164 so you might want to merge that first so the diff doesn't look so muddy. Alternatively just review the relevant commit.

Bilge added 2 commits July 5, 2016 11:51
Corrected instance calling convention in test files with static invocation.
@michalbundyra
Copy link
Member

@Bilge sorry, what warning you get? You can check last travis build and all seems to be fine.
It will be nice to use short array syntax, I'd go also with trailing commas at the end of multiline arrays. I'm not sure about changing $this-> to self:: in tests (I know that it's correct, but commonly used is $this->, also in PHPUnit documentation).

@Bilge
Copy link
Contributor Author

Bilge commented Oct 3, 2016

getMock is deprecated.

@michalbundyra
Copy link
Member

@Bilge yes, it is since PHPUnit 5.4. As you can see library required PHPUnit 4.8 so at this moment all is good.

@Bilge
Copy link
Contributor Author

Bilge commented Oct 3, 2016

@webimpress It is deprecated in the 4.x branch.

@Bilge
Copy link
Contributor Author

Bilge commented Oct 3, 2016

I would not have changed it if it wasn't necessary.

@michalbundyra
Copy link
Member

michalbundyra commented Oct 3, 2016

I would not have changed it if it wasn't necessary.

@Bilge Please check above resources and than re-thing if your change was necessary.

BTW. From ZF conduct:

To err is human. You might not intend it, but mistakes do happen and
contribute to build experience. Tolerate honest mistakes, and don't hesitate
to apologize if you make one yourself.

I think all libraries should contain this ...

@Bilge
Copy link
Contributor Author

Bilge commented Oct 3, 2016

@webimpress Please get out of my PRs your commentary is unwelcome.

@gianarb
Copy link
Contributor

gianarb commented Oct 3, 2016

Hello @Bilge , you are always too polite!

We have to take a common street across modules for this feature. I close this PR right know and I will come back soon on your work with a cherry-pick if we decide to take this street.

Thanks for your PR

@gianarb gianarb closed this Oct 3, 2016
@Bilge
Copy link
Contributor Author

Bilge commented Oct 3, 2016

@gianarb I fail to see what difficulty you have in making a decision here. Short array syntax is available since PHP 5.4, for the past 4.5 years, and the minimum supported version has already moved on to 5.6. It should be an easy decision for anyone to make.

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

3 participants