search

Nyholm / psr7

A super lightweight PSR-7 implementation
MIT License
1.17k stars 77 forks source link

Compatibility with recent PHPUnit (8) #132

Closed DavidPrevot closed 3 years ago

Zegnat commented 5 years ago

We use PHPUnit 7.5, which does not do any type definitions for setup. So is this a commit for PHPUnit 8 (8.4?) compatibility?

Here the same question arises as in #130: are we planning to drop PHP 7.1 support? That seems to be the requirement for upgrading to PHPUnit 8, which requires at least PHP 7.2 to run.

DavidPrevot commented 5 years ago

Hi,

is this a commit for PHPUnit 8 (8.4?) compatibility?

Indeed it is. This syntax is already accepted by PHPUnit 7 this project is currently using, hence this proposal: the tests still pass with the current setup, and won’t break when updating PHPUnit.

The PHP version used by this project is orthogonal to this PR, it may just make it a bit easier if it changes.

Zegnat commented 5 years ago

Would love to do this in addition to then adding || ^8.4 to the PHPUnit requirement, just so there would be some actual forward momentum. A change to return type just for change’s sake never sits well with me, though it is a perfectly valid commit.

Sadly, because of upstream php-http/psr7-integration-tests we aren’t compatible with PHPUnit 8 at all. Adding the void return type doesn’t change this.

Not sure how I’d personally progress on this, I’ll leave that for @Nyholm to decide.

GrahamCampbell commented 4 years ago

Why add this, when the composer.json file doesn't allow PHPUnit 8?

Nyholm commented 4 years ago

I don't really see any benefits for this at the moment.

Sure, we can do it as "why not". But we need to make sure the test dependencies also support PHPUnit 8