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.

Sign up for GitHub

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

Jump to bottom

[RFC] Remove LazyPromise #98

Merged
merged 1 commit into from
Aug 25, 2017
Merged

[RFC] Remove LazyPromise #98

merged 1 commit into from
Aug 25, 2017

Conversation

jsor
Copy link
Member

@jsor jsor commented Aug 25, 2017

The best code is no code

I've personally never used LazyPromise and it seems noone else really does :)

This shows exactly 2 usages, 1 in a deprecated library (icicle) and 1 wrong usage (then() is instantly called which defeats the purpose if the lazy promise).

@jsor jsor added this to the v3.0 milestone Aug 25, 2017
@jsor jsor requested review from WyriHaximus and clue August 25, 2017 08:49
WyriHaximus
WyriHaximus approved these changes Aug 25, 2017
View reviewed changes
Copy link
Member

@WyriHaximus WyriHaximus left a comment

Choose a reason for hiding this comment

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

Never used it either 👍

clue
clue approved these changes Aug 25, 2017
View reviewed changes
Copy link
Member

@clue clue left a comment

Choose a reason for hiding this comment

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

I agree that this class is misleading and have seen this attract some poor code and wrong assumptions in the past. As such, I'd also rather remove this and educate users instead 👍

@WyriHaximus WyriHaximus merged commit 177fe31 into reactphp:master Aug 25, 2017
@jsor jsor deleted the remove-lazy-promise branch August 25, 2017 09:03
@jsor jsor mentioned this pull request Jun 4, 2018
Merged
@clue clue mentioned this pull request Jun 21, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WyriHaximus WyriHaximus WyriHaximus approved these changes

@clue clue clue approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.0.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jsor @WyriHaximus @clue