Skip to content

Template params can only have one argument#35

Merged
clue merged 1 commit intoreactphp:1.xfrom
WyriHaximus-labs:1.x-template-params-can-only-have-one-argument
Jan 25, 2023
Merged

Template params can only have one argument#35
clue merged 1 commit intoreactphp:1.xfrom
WyriHaximus-labs:1.x-template-params-can-only-have-one-argument

Conversation

@WyriHaximus
Copy link
Member

@WyriHaximus WyriHaximus commented Jan 21, 2023

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
Closed
@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

WyriHaximus commented Jan 24, 2023

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

SimonFrings commented Jan 25, 2023

@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 commented Jan 25, 2023

@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@clue clue clue approved these changes

@SimonFrings SimonFrings SimonFrings 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