-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
test: refactor test-stream-transform-object #10588
Conversation
var i = -1; | ||
var int = setInterval(function() { | ||
let i = -1; | ||
const int = setInterval(function() { | ||
if (i > 10) { |
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.
Can this hardcoded number also be replaced with expect.length
?
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.
Would have to be something odd like expect.length - 2
or expect[expect.length -1]
or something like that. I'm not opposed but not exactly in favor either.
var i = -1; | ||
var int = setInterval(function() { | ||
let i = -1; | ||
const int = setInterval(function() { |
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.
It might look like overkill, but how about mustCall
ing this function as well?
* use common.mustCall() as appropriate * eliminate exit handler * var -> const/let * provide duration for setInterval()
@thefourtheye Nits addressed, I think. PTAL. |
* use common.mustCall() as appropriate * eliminate exit handler * var -> const/let * provide duration for setInterval() PR-URL: nodejs#10588 Reviewed-By: Italo A. Casas <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Landed in f43ea2a |
* use common.mustCall() as appropriate * eliminate exit handler * var -> const/let * provide duration for setInterval() PR-URL: nodejs#10588 Reviewed-By: Italo A. Casas <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
* use common.mustCall() as appropriate * eliminate exit handler * var -> const/let * provide duration for setInterval() PR-URL: nodejs#10588 Reviewed-By: Italo A. Casas <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
* use common.mustCall() as appropriate * eliminate exit handler * var -> const/let * provide duration for setInterval() PR-URL: nodejs#10588 Reviewed-By: Italo A. Casas <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
* use common.mustCall() as appropriate * eliminate exit handler * var -> const/let * provide duration for setInterval() PR-URL: nodejs#10588 Reviewed-By: Italo A. Casas <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
* use common.mustCall() as appropriate * eliminate exit handler * var -> const/let * provide duration for setInterval() PR-URL: nodejs#10588 Reviewed-By: Italo A. Casas <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
* use common.mustCall() as appropriate * eliminate exit handler * var -> const/let * provide duration for setInterval() PR-URL: nodejs#10588 Reviewed-By: Italo A. Casas <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
This does not land cleanly in LTS. Added dont-land label. Please feel free to manually backport |
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
test stream