-
-
Notifications
You must be signed in to change notification settings - Fork 26
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
Run tests on PHP 8.3 and update test suite #113
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.
Thanks for filing this pull request 👍
I added a few remarks below, should both be quick fixes.
I also noticed that the test suite currently reports unhandled promise rejections in this project (see reactphp/promise#248 and https://github.com/orgs/reactphp/discussions/525 for reference), but I think this is something for a follow-up PR. I will look into this.
Tysm @SimonFrings for seeing both mistakes! Helps me a lot for the future. 🥳 Should be alright now! |
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.
Thanks for applying the changes, looking good 👍
38cc452
to
233e5db
Compare
75372bb
to
83dce2a
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.
@yadaiio Thanks for filing this PR!
I've updated the PR to no longer report any deprecations as reported in #114. This PR now no longer reports any test errors and focuses on adding PHP 8.3 only.
I'll file separate follow-up PRs to address the deprecations reported in #114 and the unhandled promise rejections reported in #115.
Thank you @clue for the update 👍 |
This pull request builds on top of #108 #107 and #105.
References: clue/reactphp-csv#33, reactphp/socket#300 and reactphp/socket#299.
After this pull request is merged, we can continue with #112