Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Messenger] Activation middleware decorator#27320
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
[Messenger] Activation middleware decorator#27320
Uh oh!
There was an error while loading.Please reload this page.
Conversation
sroze commentedMay 30, 2018
How would someone use it? Manually, as in registers a service that |
ogizanagi commentedMay 30, 2018
@sroze : Exactly. It'll just leverage DI capabilities. |
sroze left a comment
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.
Makes sense to me 👍 (we definitely need a documentation PR for this though :))
| /** | ||
| * @param Envelope $message | ||
| */ | ||
| public function handle($message, callable $next) |
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.
PHP allows you to rename$message to$envelope here, right?
nicolas-grekas left a comment
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.
LGTM, with one naming question.
| * file that was distributed with this source code. | ||
| */ | ||
| namespace Symfony\Component\Messenger\Middleware\Enhancers; |
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.
Is theEnhancers sub-namespace required? Nesting hurts discovery IMHO, better remove it?
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.
This was mainly echoingSymfony\Component\Messenger\Transport\Enhancers which are decorators. But of course I'm fine with removing this namespace.
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'd keep it that way; it helps when exploring the code base I believe.
sroze commentedJul 8, 2018
Thank you@ogizanagi. |
This PR was merged into the 4.2-dev branch.Discussion----------[Messenger] Activation middleware decorator| Q | A| ------------- | ---| Branch? | master <!-- see below -->| Bug fix? | no| New feature? | yes <!-- don't forget to update src/**/CHANGELOG.md files -->| BC breaks? | no <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass? | yes <!-- please add some, will be required by reviewers -->| Fixed tickets | part of#26901 <!-- #-prefixed issue number(s), if any -->| License | MIT| Doc PR | TODOA small middleware decorator that can be wired using DI decoration to enable/disable a middleware on an arbitrary condition. This can be used to keep the same middleware stack across env but enable/disable some of them through this.Commits-------6e43838 [Messenger] Activation middleware decorator
kbond commentedJul 9, 2018
Is there a doc PR for this? |
ogizanagi commentedJul 9, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@kbond : Not yet. Feel free to open one if you want. Otherwise it'll go in my TODO list anyway :) EDIT: I've createdsymfony/symfony-docs#10035 to help |
Uh oh!
There was an error while loading.Please reload this page.
A small middleware decorator that can be wired using DI decoration to enable/disable a middleware on an arbitrary condition. This can be used to keep the same middleware stack across env but enable/disable some of them through this.