Conversation
|
@wanlin31 @XuanWang-Amos @alto-php I wonder if there is anyone who could look at this 🙇♂️ |
src/php/composer.json
Outdated
| "version": "1.73.0", | ||
| "require": { | ||
| "php": ">=7.0.0", | ||
| "php": ">=8.1.0", |
There was a problem hiding this comment.
since the typing of a nullable functional parameter was introduced in 7.1, minimum compatible version with this syntax is 7.1. I also noticed the versions under 8.0 have reached EOL. So, I bumped the version to 8.1. But considering the impact, should we maximize acceptable versions?
|
I think the same change should also be done to ServerCallWriter::addSendInitialMetadataOpIfNotSent |
|
@chschneider nice catch, thank you! |
|
Hi, do we know when this changes will be merged? Thank you 🙇🏽 |
|
waiting on this |
|
@stanley-cheung wouldn't you be able to take a look at this PR? |
| "version": "1.73.0", | ||
| "require": { | ||
| "php": ">=7.0.0", | ||
| "php": ">=7.1.0", |
There was a problem hiding this comment.
Sorry there's some non-obvious things about the code base here, where some template files need to match up.
Can you please make the same change in this file as well? https://github.com/grpc/grpc/blob/master/templates/src/php/composer.json.inja#L7
Thanks.
|
This is merged. Thanks for the contributions. This will go onto the next 1.74.0 release which should happen in the next couple of weeks. |
|
Wohoo, This is great news. Thank you I have a strong recommendation for 1.74.0: This PR: #37563 It adds one line of comment (not code). This will help static analyzer tools like PHPStan to understand the types which means I will write fewer bugs in my application. That would be a BIG improvement. |
|
@stanley-cheung thank you so much for the support! could you take a look at this PR in the mirror repository after the release? grpc/grpc-php#38 |
From PHP8.4, a parameter that has `null` as a default value has to be typed as a nullable type: https://www.php.net/manual/en/migration84.deprecated.php In the code base, the factory methods of `Status` class have to be fixed to comply with this syntax. On the other hand, nullable type is not available in 7.0 (https://www.php.net/manual/en/migration71.new-features.php), which is supported in this library now. <strike>Since the versions 7.0 - 8.0 have already reached the EOL (https://www.php.net/supported-versions.php) as of now, I would like to propose dropping support of these versions, and adding support of 8.4.</strike> cc: @stanley-cheung Closes grpc#39199 COPYBARA_INTEGRATE_REVIEW=grpc#39199 from shu-yusa:master 29cc63f PiperOrigin-RevId: 775306237
From PHP8.4, a parameter that has `null` as a default value has to be typed as a nullable type: https://www.php.net/manual/en/migration84.deprecated.php In the code base, the factory methods of `Status` class have to be fixed to comply with this syntax. On the other hand, nullable type is not available in 7.0 (https://www.php.net/manual/en/migration71.new-features.php), which is supported in this library now. <strike>Since the versions 7.0 - 8.0 have already reached the EOL (https://www.php.net/supported-versions.php) as of now, I would like to propose dropping support of these versions, and adding support of 8.4.</strike> cc: @stanley-cheung Closes grpc#39199 COPYBARA_INTEGRATE_REVIEW=grpc#39199 from shu-yusa:master 29cc63f PiperOrigin-RevId: 775306237
From PHP8.4, a parameter that has
nullas a default value has to be typed as a nullable type: https://www.php.net/manual/en/migration84.deprecated.phpIn the code base, the factory methods of
Statusclass have to be fixed to comply with this syntax. On the other hand, nullable type is not available in 7.0 (https://www.php.net/manual/en/migration71.new-features.php), which is supported in this library now.Since the versions 7.0 - 8.0 have already reached the EOL (https://www.php.net/supported-versions.php) as of now, I would like to propose dropping support of these versions, and adding support of 8.4.cc: @stanley-cheung