Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Template params can only have one argument #35

Merged
merged 1 commit into from
Jan 25, 2023
Merged

Template params can only have one argument #35

merged 1 commit into from
Jan 25, 2023

Conversation

WyriHaximus
Copy link
Member

The fact that a promise can also be rejected with a Throwable and/or Exception is implied and there is no need to also define that here.

Refs: reactphp/promise#223

@WyriHaximus WyriHaximus added this to the v1.6.0 milestone Jan 21, 2023
@WyriHaximus WyriHaximus mentioned this pull request Jan 21, 2023
Draft
@clue clue added maintenance and removed bug labels Jan 24, 2023
clue
clue requested changes Jan 24, 2023
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.

Just noticed this should also be applied to the README documentation. @WyriHaximus is this something you can look into? 👍

@WyriHaximus WyriHaximus force-pushed the 1.x-template-params-can-only-have-one-argument branch from eba153c to 709e36b Compare January 24, 2023 16:04
@WyriHaximus WyriHaximus requested a review from clue January 24, 2023 16:05
@WyriHaximus
Copy link
Member Author

Just noticed this should also be applied to the README documentation. @WyriHaximus is this something you can look into? +1

@clue Just updated the README documentation and amended those changes to this PR.

@SimonFrings
Copy link
Member

@WyriHaximus Same should also be applied to the UnwrapWritableStream.php and UnwrapReadableStream.php files, right?

@WyriHaximus
Template params can only have one argument
b606aec 
The fact that a promise can also be rejected with a Throwable and/or Exception is implied and there is no need to also define that here.

Refs: reactphp/promise#223
@WyriHaximus WyriHaximus force-pushed the 1.x-template-params-can-only-have-one-argument branch from 709e36b to b606aec Compare January 25, 2023 07:20
@WyriHaximus
Copy link
Member Author

@WyriHaximus Same should also be applied to the UnwrapWritableStream.php and UnwrapReadableStream.php files, right?

@SimonFrings Hah yeah I assumed we only do PromiseInterface< in the packages not Promise< as well. Updated those two files as well and doing a search through the other packages as well to be sure I didn't miss anything else.

@clue clue merged commit 2806f6f into reactphp:1.x Jan 25, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@SimonFrings SimonFrings SimonFrings approved these changes

@clue clue clue approved these changes

Assignees
No one assigned
Labels
maintenance
Projects
None yet
Milestone
v1.6.0
Development

Successfully merging this pull request may close these issues.
3 participants
@WyriHaximus @SimonFrings @clue