Skip to content

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