Skip to content
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

Simplify App by always using default loop, drop optional loop instance #88

Merged
merged 1 commit into from
Nov 30, 2021

Conversation

clue
Copy link
Owner

@clue clue commented Nov 30, 2021

This changeset simplifies the App by removing the optional $loop argument and always using the default loop. Among others, this simplifies usage, improves the API documentation, removes a bunch of ambiguous tests, and overall helps clean up the project. Overall, we have yet to see a use case that requires an explicit loop instance and given that fibers require a single, shared loop instance anyway (reactphp/async#15), it's unlikely we'll find many use cases for this.

This is obviously a BC break for consumers that currently explicitly pass a loop instance, but it is believed that this should not affect most consumers of this package. If you are affected, updating this is very straightforward:

// old
$loop = React\EventLoop\Loop::get();
$app = new FrameworkX\App($loop); 

// new (already supported before)
$app = new FrameworkX\App();

Builds on top of #22, #29 and #81

@clue clue added new feature New feature or request BC break labels Nov 30, 2021
@SimonFrings SimonFrings merged commit 5e86f28 into clue:main Nov 30, 2021
@clue clue deleted the default-loop branch November 30, 2021 14:16
@SimonFrings SimonFrings added this to the v0.5.0 milestone Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BC break new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants