-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
Update to require PHP 7.1+ #222
Conversation
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.
Looks good overall, but found some points worth addressing.
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.
@SimonFrings Thanks for looking into this! Excellent work so far, only spotted some minor oversights below.
Also noticed that there are still a number of assertInstanceOf()
calls with class name strings. I would suggest updating this to the class syntax (StringClassNameToClassConstantRector
is your friend) while we're at it as suggested in reactphp/stream#175 (review).
41f11e0
to
edfcecd
Compare
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.
@SimonFrings Thanks for the update, changes LGTM!
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.
Sweet, thanks for updating everything 👍
This changeset improves PHP 8.4+ support by avoiding implicitly nullable types as discussed in reactphp/promise#260. I'm planning to add native types to the public API and introduce PHPStan in follow-up PRs. Once merged, we should apply similar changes to all our upcoming v3 components. On top of this, we should backport similar changes to the v1 branch. Builds on top of reactphp#182, reactphp#222 and reactphp/promise#260
This changeset adds native types to the public API as discussed in reactphp#219. Once merged, I'm planning to add PHPStan in a follow-up PR which would take advantage of these types. Builds on top of reactphp#222, reactphp#223 and reactphp/cache#60
This changeset adds native types to the public API as discussed in reactphp#219. Once merged, I'm planning to add PHPStan in a follow-up PR which would take advantage of these types. Builds on top of reactphp#222, reactphp#223 and reactphp/cache#60
This changeset improves PHP 8.4+ support by avoiding implicitly nullable types as discussed in reactphp/promise#260. I'm planning to add native types to the public API and introduce PHPStan in follow-up PRs. Once merged, we should apply similar changes to all our upcoming v3 components. On top of this, we should backport similar changes to the v1 branch. Builds on top of reactphp#182, reactphp#222 and reactphp/promise#260
This changeset improves PHP 8.4+ support by avoiding implicitly nullable types as discussed in reactphp/promise#260. I'm planning to add native types to the public API and introduce PHPStan in follow-up PRs. Once merged, we should apply similar changes to all our upcoming v3 components. On top of this, we should backport similar changes to the v1 branch. Builds on top of reactphp#182, reactphp#222 and reactphp/promise#260
This changeset improves PHP 8.4+ support by avoiding implicitly nullable types as discussed in reactphp/promise#260. I'm planning to add native types to the public API and introduce PHPStan in follow-up PRs. Once merged, we should apply similar changes to all our upcoming v3 components. On top of this, we should backport similar changes to the v1 branch. Builds on top of reactphp#182, reactphp#222 and reactphp/promise#260
This changeset adds native types to the public API as discussed in reactphp#219. Once merged, I'm planning to add PHPStan in a follow-up PR which would take advantage of these types. Builds on top of reactphp#222, reactphp#223 and reactphp/cache#60
This changeset adds native types to the public API as discussed in reactphp#219. Once merged, I'm planning to add PHPStan in a follow-up PR which would take advantage of these types. Builds on top of reactphp#222, reactphp#223 and reactphp/cache#60
This changeset adds native types to the public API as discussed in reactphp#219. Once merged, I'm planning to add PHPStan in a follow-up PR which would take advantage of these types. Builds on top of reactphp#222, reactphp#223 and reactphp/cache#60
This changeset adds native types to the public API as discussed in reactphp#219. Once merged, I'm planning to add PHPStan in a follow-up PR which would take advantage of these types. Builds on top of reactphp#222, reactphp#223 and reactphp/cache#60
This changeset adds native types to the public API as discussed in reactphp#219. Once merged, I'm planning to add PHPStan in a follow-up PR which would take advantage of these types. Builds on top of reactphp#222, reactphp#223 and reactphp/cache#60
This changeset adds native types to the public API as discussed in reactphp#219. Once merged, I'm planning to add PHPStan in a follow-up PR which would take advantage of these types. Builds on top of reactphp#222, reactphp#223 and reactphp/cache#60
This changeset adds native types to the public API as discussed in reactphp#219. Once merged, I'm planning to add PHPStan in a follow-up PR which would take advantage of these types. Builds on top of reactphp#222, reactphp#223 and reactphp/cache#60
This changeset updates the project to require PHP 7.1+ and drop legacy PHP < 7.1 and HHVM as discussed in #219. I'm marking this as a BC break for anybody still stuck on very old PHP versions, but there's little chance this will affect any current projects otherwise.
This PR aims to contain the minimal changeset to update the PHP version requirement only. Follow-up PRs will update our APIs to leverage newer language features.
FYI: I used reactphp/cache#58, reactphp/stream#175 and reactphp/event-loop#276 as inspiration.
Builds on top of #101, #170, #217, #220 and others