-
Notifications
You must be signed in to change notification settings - Fork 30k
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
doc: add timeout.close #40036
doc: add timeout.close #40036
Conversation
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 for opening this!
Does this have any benefit over |
this is a wrapper around clearTimeout. The name doesn't have |
Actually the legacy status is there for features that we likely won't remove ever: Lines 42 to 44 in 6fdd582
According to the collaborator guide, if it's a feature that was exposed and we have good reasons to think it's used out there (in this case we know for sure it is), we should document it as public: node/doc/guides/collaborator-guide.md Lines 313 to 317 in 7dbc0f7
Since it's almost an alias, I think the following applies: node/doc/guides/collaborator-guide.md Lines 442 to 443 in 7dbc0f7
In this case, I personally it should be documented with Legacy status, unless there are use cases where it makes sense to use it over clearTimeout (in which case it should be documented as stable).
|
@jasnell, could you approve this pull? |
Landed in c365145...de10ab2 |
PR-URL: #40036 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Michaël Zasso <targos@protonmail.com>
PR-URL: #40036 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Michaël Zasso <targos@protonmail.com>
PR-URL: #40036 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Michaël Zasso <targos@protonmail.com>
Hello,
I noticed that the
Timeout.close
is not documented. We have this method from v0.9.1:node/lib/timers.js
Lines 274 to 282 in 985e3a2
Current codebase wraps clearTimeout:
node/lib/timers.js
Lines 251 to 254 in c4096a3
This pull adds documentation for this method.