feat: Enable the usage of "psr/http-message": "^1.0|^2.0"
#6338
Conversation
jeromegamez
force-pushed
the
psr-http-message-2
branch
2 times, most recently
from
4e0e415 to
4d09ab1
Compare
jeromegamez
force-pushed
the
psr-http-message-2
branch
from
4d09ab1 to
1c87614
Compare
Versions 5.26.5|6.9.6|7.3.1 remove `psr/http-message` from their direct dependencies to not block the installation of more recent versions.
jeromegamez
force-pushed
the
psr-http-message-2
branch
from
1c87614 to
619f077
Compare
| @@ -47,7 +47,7 @@ public function __construct(StreamInterface $stream) | |||
| * | |||
| * @return int The size of the stream. | |||
| */ | |||
| public function getSize() | |||
| public function getSize(): ?int | |||
There was a problem hiding this comment.
won't these changes break compatibility with psr/http-message: ^1.0? Have you tested it with this version of psr/http-message? I think it's fine to accept a breaking change here, but if it doesn't work with 1.0 we should remove that version from composer.json
edit: I tested with psr/http-message:1.0 and it works fine
There was a problem hiding this comment.
Hmm, realizing now that these changes are in Storage and not in Debugger, so we need to be very careful about what we do here as far as breaking customers.
There was a problem hiding this comment.
The reason this is considered a breaking change is because if someone has made a subclass of ReadStream or WriteStream, and they defined the getSize() method without a return type, then this change would result in a PHP fatal error :
Declaration of SomeCustom\ReadStream::getSize() must be compatible with Google\Cloud\Storage\ReadStream::getSize(): ?int
To avoid this being a breaking change, we would want to add final to the class, or even an @internal label, to signal that it's not meant to be subclassed.
In general I think these changes are benign. I don't think anyone is subclassing these, and if they are, an easy fix would be to just add the return types to the subclassed methods.
| @@ -47,7 +47,7 @@ public function __construct(StreamInterface $stream) | |||
| * | |||
| * @return int The size of the stream. | |||
| */ | |||
| public function getSize() | |||
| public function getSize(): ?int | |||
There was a problem hiding this comment.
Hmm, realizing now that these changes are in Storage and not in Debugger, so we need to be very careful about what we do here as far as breaking customers.
|
Adding @saranshdhingra because we need his approval on the "breaking changes" before merging |
bshaffer
added
do not merge
|
Thank you for merging 🙏🏻🌺 |
Closes #6243
Notes
kreait/firebase-phpremoves the direct dependency topsr/http-message, which prevented the usage of newer versions (https://github.com/kreait/firebase-php/releases/tag/5.26.5)BREAKING_CHANGE_REASON=Covariant return types are supported starting with PHP 7.4. The added return types only narrow down the return types. (https://wiki.php.net/rfc/covariant-returns-and-contravariant-parameters)