Skip to content

[Messenger] Uses the messenger.message_handler tag instead of message.handler #26676

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

Merged

Conversation

sroze
Copy link
Contributor

@sroze sroze commented Mar 26, 2018

Q A
Branch? master
Bug fix? yes
New feature? no
BC breaks? no (technically, it's not even in 4.1)
Deprecations? no
Tests pass? yes
Fixed tickets ø
License MIT

After a few days using it, it feels weird to have the messenger.receiver and messenger.sender tags while the one for message handlers is message_handler. I believe that for consistency, this would make sense to rename this tag messenger.message_handler.

As we don't have tests for this pass yet (will be added in #26672) there is nothing else to change (but the documentation).

@ogizanagi
Copy link
Contributor

Why not messenger.handler? I don't think there is any ambiguity here, right?

@sroze
Copy link
Contributor Author

sroze commented Mar 26, 2018

Why not messenger.handler?

Why not. I like this option as well. Any other thoughts?

@nicolas-grekas nicolas-grekas added this to the 4.1 milestone Mar 27, 2018
@nicolas-grekas
Copy link
Member

messenger.message_handler looks like the most explicit to me, and opens for more kind of handlers in the futur, if needed.

@fabpot
Copy link
Member

fabpot commented Mar 27, 2018

Thank you @sroze.

@fabpot fabpot merged commit bddebc4 into symfony:master Mar 27, 2018
fabpot added a commit that referenced this pull request Mar 27, 2018
…tead of `message.handler` (sroze)

This PR was merged into the 4.1-dev branch.

Discussion
----------

[Messenger] Uses the `messenger.message_handler` tag instead of `message.handler`

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no (technically, it's not even in 4.1)
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | ø
| License       | MIT

After a few days using it, it feels weird to have the `messenger.receiver` and `messenger.sender` tags while the one for message handlers is `message_handler`. I believe that for consistency, this would make sense to rename this tag `messenger.message_handler`.

As we don't have tests for this pass yet (will be added in #26672) there is nothing else to change (but the documentation).

Commits
-------

bddebc4 Uses the `messenger.message_handler` tag instead of `message.handler`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants