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

docs(events): add events listener options #2888

Merged

Conversation

koreanddinghwan
Copy link
Contributor

@koreanddinghwan koreanddinghwan commented Oct 18, 2023

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Docs
  • Other... Please describe:

What is the current behavior?

  • too few information about event-listener options

What is the new behavior?

event listener options object updated at feat: add suppress errors option
i think need more information to users who use events.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@@ -94,6 +117,9 @@ handleOrderCreatedEvent(payload: OrderCreatedEvent) {
}
```

> info **Hint** read more about ([listener options object](https://github.com/EventEmitter2/EventEmitter2#emitteronevent-listener-options-objectboolean)).
Copy link
Member

@micalevisk micalevisk Oct 18, 2023

Choose a reason for hiding this comment

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

Suggested change
> info **Hint** read more about ([listener options object](https://github.com/EventEmitter2/EventEmitter2#emitteronevent-listener-options-objectboolean)).
> info **Hint** Read more about the [`OnOptions` options object from `eventemitter2`](https://github.com/EventEmitter2/EventEmitter2#emitteronevent-listener-options-objectboolean).

what about this? I'd like to clarify from where that OnOptions came

Copy link
Member

Choose a reason for hiding this comment

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

also, can we move this HINT to right below L111?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think that it is better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, move to below L111

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@koreanddinghwan koreanddinghwan changed the title docs(events): add events listner options docs(events): add events listener options Oct 18, 2023
@kamilmysliwiec kamilmysliwiec merged commit 09095ad into nestjs:master Oct 23, 2023
4 checks passed
@koreanddinghwan koreanddinghwan deleted the docs/event-listener-options branch October 24, 2023 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants