[v12.x] readline: revert semver-major of 28109#28157
[v12.x] readline: revert semver-major of 28109#28157sam-github wants to merge 2 commits intonodejs:v12.x-stagingfrom
Conversation
It was intended, according to in-test comments and common behaviour, that callbacks be either `undefined` or a function, but falsy values were being accepted as meaning "no callback". PR-URL: nodejs#28109 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
Sadly, an error occurred when I tried to trigger a build. :( |
|
Hm, in this specific case backporting the change does indeed not seem to provide much benefit. We do not yet have any example around it but that's a good idea and I'll raise an issue in the release working group about it! I'll go ahead and close this, but thanks a lot for doing this nevertheless! |
|
As you wish, though notice it does add regression checks for a current (odd) behaviour. |
|
Not to say it should land... I'm on the fence, too. |
|
Hm, I would be fine with landing that regression test as well but I guess it's not required on 12 since master has a test for it and the only thing that could break it should likely be a bad backport (or a Node.js 12 specific PR but those are rare)? |
|
Refs: nodejs/Release#459 |
3 participants
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes