-
Notifications
You must be signed in to change notification settings - Fork 30.5k
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
readline.question #4833
Comments
It's used by about 0.3% of modules, including babel, pm2, webdriverio, replace, atom-package-manager. |
Yes, it's definitely used in quite a bit of code. Still, the inconsistency bugs me. I wonder if soft deprecation would work here if it's even something we're interested in. |
I've used it and it is useful. It is a bit odd I suppose, but you would never catch an error there afaik. It simply doesn't make any sense to me why there would ever be one. |
Well, even a method that doesn't raise errors (and whether or not I admit it's useful and I'm not for removing it. Maybe put a big note in the docs saying it's inconsistenct with other node style APIs? |
I wouldn't be against putting a note, but I don't think it should actually have an errback, since it would always be undefined. |
Is there a synchronous substitute of |
Clarify that readline's rl.question() callback does not use the err back pattern. Fixes: nodejs#4833
Good job closing this! |
Clarify that readline's rl.question() callback does not use the err back pattern. Fixes: #4833 PR-URL: #7022 Reviewed-By: Benjamin Gruenbaum <[email protected]>
So, I found readline.question today, and it looks really odd.
First, it doesn't follow the
(err, data)
nodeback convention at all (where do errors go?).Second, it doesn't really feel like something that should be in core.
Should it be soft-deprecated?
Should it be "fixed" to conform to the node-back convention (not very likely)?
The text was updated successfully, but these errors were encountered: