-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Improve CQRS event handlers doc #2476
Comments
@Tony133 No offense, but could you let met create that PR please ? |
Hi @CDelgrange, no problem, as soon as you do the PR I close mine 👍 No, an issue does not have to be approved, to create a PR. If you want future contributions to the NestJS repositories you directly open a PR with what you think needs to be changed, without creating an issue, then it will be evaluated and approved in code review ,etc Let's say it is a "faster" step. |
Thanks a lot @Tony133 👌 Noted ! :) |
You're welcome, it's nothing. 😉👍 I closed my PR |
This issue we can close since the merge has been done here: #2486 |
You're right 👍 |
Is there an existing issue that is already proposing this?
Is your feature request related to a problem? Please describe it
The "problem" is already known, you can find details in this issue.
TLDR: When you start using CQRS events / event handlers, you get out of the traditional HTTP web context. There are multiple solutions to solve this behavior, but the point here, is that it's not documented anywhere and it can lead to unexpected crashes.
Describe the solution you'd like
A simple Hint in the doc explaining that behavior and what to expect in order to react appropriately.
Teachability, documentation, adoption, migration strategy
Here is what I would like to add to the doc:
What is the motivation / use case for changing the behavior?
It's not changing the behavior, it's just inform people for better communication and some time saved for everyone.
The text was updated successfully, but these errors were encountered: