-
-
Notifications
You must be signed in to change notification settings - Fork 406
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
New command - make:message for Messenger #338
Conversation
a9e46bf
to
891552e
Compare
class <?= $class_name."\n" ?> | ||
{ | ||
/* | ||
* Add whatever properties & methods you need to hold the |
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.
what about doing it like in the make entity command, one could nearly reuse the existing code without the annotations stuff. This would make it more handy imo
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.
Hi, can you point what part of command or entity can be reused? :)
|
||
namespace <?= $namespace; ?>; | ||
|
||
class <?= $class_name."\n" ?> |
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.
would you mind making this final
?
use <?= $message_full_class_name ?>; | ||
use Symfony\Component\Messenger\Handler\MessageHandlerInterface; | ||
|
||
class <?= $class_name ?> implements MessageHandlerInterface |
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.
final
, too?
// return $this->name; | ||
// } | ||
// | ||
// public function setName(string $name) |
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.
Would be preferable to keep messages immutable. What about not promoting setters?
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 idea 💡
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 agree to remove the setter example
* Note - if you're handling this message async, you may need | ||
* a getter & setter method for each property so that it serializes | ||
* and unserializes correctly. | ||
*/ |
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 comment can now be removed
// return $this->name; | ||
// } | ||
// | ||
// public function setName(string $name) |
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 agree to remove the setter example
22348e9
to
e34533a
Compare
Hi, according to a chat i had with @weaverryan i've completed this maker in order to allow the user to route the message to a transport |
I am routing my Messages based on an „AsyncMessage“-Marker Interface, is it worth to add such an option instead of manipulating the messenger.yaml file? |
It’s a very interesting idea... but I don’t think it’s standard enough yet to put into maker. Better to show them where/how routing happens |
i kinda agree with @weaverryan. Although it is a nice trick, it may involve too much complexity and different cases to handle in the maker. BTW, the CI is failing because of some deprecation i fixed here: #518 |
e34533a
to
a082c1e
Compare
a082c1e
to
2ee1c06
Compare
Thank you for taking this across the finish line @nikophil! |
Nice, when can we expect a release? 😊😎 |
Soon! Finishing the new reset password stuff! |
An easy one - generated for Messenger - generates an empty message class and corresponding handler:
Generated Code
SendWelcomeEmailMessage
SendWelcomeEmailMessageHandler
In the future... we could possibly ask which transport you want to use and update your YAML configuration (if needed) 🤔