Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

documentation: emitter.setMaxListeners(Infinity) #25269

Closed
wants to merge 1 commit into from
Closed

documentation: emitter.setMaxListeners(Infinity) #25269

wants to merge 1 commit into from

Conversation

jasonkarns
Copy link
Member

Instead of recommending 0 as the magic value to set max listeners to unlimited, recommend Infinity.

closes #22987

Instead of recommending `0` as the magic value to set max listeners to unlimited, recommend `Infinity`.

closes #22987
@jasonkarns jasonkarns changed the title document Infinity for emitter.setMaxListeners documentation: emitter.setMaxListeners(Infinity) May 8, 2015
@jasnell jasnell added the doc label Aug 14, 2015
@jasnell
Copy link
Member

jasnell commented Aug 14, 2015

@misterdjules ... I'm inclined to close this as the documentation does accurately reflect the implementation and conventional use. Wanted to check with you before I did, however.

@jasonkarns
Copy link
Member Author

It reflects the implementation, but I argue that the implementation is poor. Intending 0 to represent Infinity when there is already the Infinity constant is nonsense. Of course, it doesn't make sense at this point to change the implementation; but changing the documentation to reflect a more meaningful value (that works functionally identical) can pave the way for the implementation to be improved in the future.

@jasnell
Copy link
Member

jasnell commented Aug 15, 2015

That may very well be the case. Currently, the right place to pursue a
change would be in the new repo. Perhaps open a new issue or PR against
http://github.com/nodejs/node?
On Aug 15, 2015 6:06 AM, "Jason Karns" [email protected] wrote:

It reflects the implementation, but I argue that the implementation is
poor. Intending 0 to represent Infinity when there is already the Infinity
constant is nonsense. Of course, it doesn't make sense at this point to
change the implementation; but changing the documentation to reflect a more
meaningful value (that works functionally identical) can pave the way for
the implementation to be improved in the future.


Reply to this email directly or view it on GitHub
#25269 (comment).

@jasonkarns
Copy link
Member Author

closing in favor of nodejs/node#2559

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants