-
-
Notifications
You must be signed in to change notification settings - Fork 836
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
feat: conditional extenders #3759
Conversation
Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been considering this for a while now actually. Great job 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job. Please do cleanups in different pr's next time, makes reviewing a ton easier 🙈
/** | ||
* @param ExtenderInterface[] $extenders | ||
*/ | ||
public function whenExtensionEnabled(string $extensionId, array $extenders): self |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The signature doesn't match the PR comment example. Your example uses a callable as the second argument, not an array. The array is the return response from the callable yes. I assume your example is wrong then..
Also the phpdoc is incorrect, there are two args.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea updated the example thanks
phpdoc doesn't matter, we only add typings for what isn't typed, we can't vanilla type array as ExtenderInterface[], we've being doing this since 1.0 to avoid unnecessary duplication. And the phpdoc standard allows it.
Changes proposed in this pull request:
Adds support for conditional extenders, needed especially when adding extenders when certain extensions are enabled. (needed for tag mentions).
Here are some usage examples:
Necessity
Confirmed
composer test
).Required changes: