Make it more obvious what the $handler is on Messenger docs #13349
Conversation
|
@Nyholm does this proposed change look good to you? Thanks! |
There was a problem hiding this comment.
I like this change because we don't use an undefined variable $handler anymore.
The MyMessageHandler is explained in the very next sentence after the code block, so I think it is fine.
This issue is present on other versions of the documentation but the behaviour of the function also changed (parameter now can be an array)
I believe we should only document the normal and simple case where we have a callable class.
This code block you changed is the first one. We can assume the reader does not know anything about the Messenger component. Where are the other places you refer to? Can one assume that the reader knows what $handler is without us telling them?
|
Hi @Nyholm I refer to the available documentation for the component since it's release. It changed 3 times until getting into the current state: I don know what we want to do to document this. If needed I can go and make PRs aiming each of the versions and document Answering your question about $handler: I considered adding a comment "see next item" or something similar when declaring the Let me know how you want me to proceed with this PR, I am happy with any decision made about the comments and the number of PRs needed to close the issue. |
|
Excellent question. The docs team is super of moving changes to different branches. I know that this will be merged in master and "copied" down to 5.0 and 4.4. We dont bother about 4.1, 4.2 and 4.3 since those versions are not maintained anymore.
I think the current solution is fine for now. I think this PR is ready for merge. @jpjoao thank you for this. No more changes is needed at the moment. 👍 |
a89e32e to
41d017a
Compare
|
@jpjoao merged! Thanks ... and congrats on your first Symfony Docs contribution! |
fixes #13148
As mentioned in the issue
$handlerwas not obvious. I referenced the next item in the docs in the hope that this makes things more clear.This issue is present on other versions of the documentation but the behaviour of the function also changed (parameter now can be an array). I am unsure of how to proceed in such a case.