Skip to content

Conversation

@yadaiio
Copy link

@yadaiioyadaiio commented Apr 23, 2024

Builds on top of #32, #37 and #39.

References: reactphp/socket#300, clue/reactphp-zenity#63, clue/reactphp-csv#33 and others.

The tests were failing in PHP 5.3 because of an unknown React\Promise\Timer\sleep() function. In order to avoid this I updated this to use the React\Async\delay() function instead.

Additionally I saw the dynamic properties were deprecated since PHP 8.2, see https://www.php.net/releases/8.2/en.php#deprecate_dynamic_properties, so I had to add a new class variable in the CompositeConnection.php file to avoid that the test for PHP 8.2 and PHP 8.3 run infinitely.

@SimonFringsSimonFrings added the new feature New feature or request label Apr 23, 2024
@SimonFringsSimonFrings requested review from SimonFrings and clue and removed request for clueApril 23, 2024 13:42
@yadaiioyadaiio changed the title Run tests on PHP 8.3 and update test suiteRun tests on PHP 8.3, fix dynamic property for PHP 8.2 and update test suiteApr 25, 2024
Copy link
Contributor

@SimonFringsSimonFrings left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dynamic property seems to be an oversight from #37, thanks for fixing this 👍

@yadaiioyadaiioforce-pushed the test_suite branch 2 times, most recently from 06215b9 to 68edc2cCompareJune 10, 2024 10:02
@yadaiioyadaiio requested review from SimonFrings and clueJune 10, 2024 10:30
@yadaiio
Copy link
Author

As today is my last day working with you @clue, we agreed closing this pull request and revisit this later on.

If anyone is needing these changes earlier than expected, please consider sponsoring @clue via https://github.com/sponsors/clue. ❤️

@yadaiioyadaiio closed this Jun 28, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new featureNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@yadaiio@clue@SimonFrings