-
Notifications
You must be signed in to change notification settings - Fork 82
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
Add note to handle events as quickly as possible. #405
Conversation
58e1ec2
to
62b0a2a
Compare
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.
Thanks!
Do you mind adding a paragraph on general operation/usage of Node
in the doc comment of struct Node
, which then should also explictly state that events need to be handled? Let me know if you prefer me writing it though, happy to it in a follow-up.
I am not sure of the extent of this "how to handle events in ldk-node" doc on top of |
/// **Caution:** Users must handle events as quickly as possible to prevent a large event backlog, | ||
/// which can increase the memory footprint of [`Node`]. |
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.
Consider styling the message:
/// <div class="warning">
///
/// Users must handle events as quickly as possible to prevent a large event backlog,
/// which can increase the memory footprint of [`Node`].
///
/// </div>
https://doc.rust-lang.org/rustdoc/how-to-write-documentation.html#adding-a-warning-block
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.
We currently don't do that everywhere else, so for now landed this PR as-is. But might be an improvement to consider in the future, which we'd then however want to apply globally across all docs.
No worries! |
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.
LGTM
No description provided.