Skip to content
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

[Messenger] Document stamps in HandleTrait::handle #20851

Merged
merged 1 commit into from
Apr 7, 2025

Conversation

Fan2Shrek
Copy link
Contributor

Fix #20842

@carsonbot carsonbot added this to the 7.3 milestone Apr 2, 2025
@Spomky
Copy link
Contributor

Spomky commented Apr 2, 2025

Hi @Fan2Shrek,

This looks good to me.
I would have added the example showed in symfony/symfony#42124 (comment) as well (it is more relevant to me compared to the DoctrineFlushStamp stamp)

Copy link
Contributor

@OskarStark OskarStark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After my comment

messenger.rst Outdated
@@ -2512,6 +2512,15 @@ wherever you need a query bus behavior instead of the ``MessageBusInterface``::
}
}

You also can also specify stamps when handling a message. For example, you can
Copy link
Member

@javiereguiluz javiereguiluz Apr 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does specify mean here? Is it about adding new stamps to the message? What if you add a stamp of the same type that some existing stamp: is it appended or overwritten? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good points! I committed some refinements. Does it look good to you?

@carsonbot carsonbot changed the title Document stamps in HandleTrait::handle [Messenger] Document stamps in HandleTrait::handle Apr 2, 2025
Copy link
Contributor

@Spomky Spomky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine for me👌.
Many thanks.

@javiereguiluz javiereguiluz merged commit 59836aa into symfony:7.3 Apr 7, 2025
3 checks passed
@javiereguiluz
Copy link
Member

Merged! Thanks Pierre.

@@ -2512,6 +2512,20 @@ wherever you need a query bus behavior instead of the ``MessageBusInterface``::
}
}

You also can also add stamps when handling a message. For example, you can
add an ``DoctrineFlushStamp`` to flush the entity manager after handling the message::
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stamp doss not exists in the symfony core maybe we should alternative or describe it as MyCustomStamp.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I'll take a look at it later today 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Alexander! I wasn't aware of this.

We updated the example directly in 7f9f20f but if you think this needs further changes, please create a Pull Request. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @javiereguiluz 👍

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.

[Messenger] Add $stamps parameter to HandleTrait::handle
6 participants