-
Notifications
You must be signed in to change notification settings - Fork 3
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
PHPUnit 6 compatibility #11
Conversation
@ascii-soup, thanks for your PR! By analyzing the history of the files in this pull request, we identified @sjparkinson to be a potential reviewer. |
I neglected to update the php version constraint in composer.json - this now needs to be 7.0 as that is what phpunit 6 requires. Addressing now... |
It seems HHVM is no longer supported on the current Ubuntu version used by Travis. Having not really used Travis much, I'm not sure if I should be updating something here or what... Let me know what needs doing and I'll fix it up. |
Yep this looks cracking, but as you say a bump to |
I think #10 needs to be addressed too, in order for this to be considered "fully compatible" so I'll have a crack at that and see what happens. Watch this space. |
With Travis, this looks like a solid guide https://docs.travis-ci.com/user/languages/php/#HHVM-versions. |
Though I don't believe it'll need sudo, just the |
It seems we are hitting this issue: composer/composer#4976 Not sure if there's anything to be done about that .... thoughts? |
|
Or even just drop support, v2 works for that. |
It's not looking brilliant on the #10 front - sebastianbergmann/phpunit#2744 Perhaps I should've worded the issue differently. Anyway, we'll see if he comes back on that front, otherwise the only option at the moment is to disable the risky test checking when using hamcrest - https://phpunit.de/manual/current/en/risky-tests.html |
Yep depending on the response worth an update to README.md. |
Yeah I've made the update to it, just waiting on a response before push.
…On 26 Jul 2017 11:35, "Samuel Parkinson" ***@***.***> wrote:
Yep depending on the response worth an update to README.md.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#11 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAmTyad5xeLqg82YdGzLUVv9EkwZs0Z_ks5sRxZXgaJpZM4OiZ2o>
.
|
README updated, having not had a response on the PHPUnit issue. If anything changes, we can make the necessary updates but for now, I think this is sufficient for PHPUnit 6 compatibility. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cracking 👌
This addresses #9 with PHPUnit 6 compatibility. As mentioned in the issue, this is a breaking change as the underlying framework classes now use namespaces.