modified test-repl-persistent-history to use common.mustCall #12703
Conversation
There was a problem hiding this comment.
Could you leave this as it was please.
There was a problem hiding this comment.
I'll do so. I removed the function because this way runTest was accounting for an additional call. I thought it'd be an extra burden on the reader if he sees common.mustCall(runTest, numtests + 1) later on. Sorry, this was my first contribution on the git via NodeTodo, I am figuring out how to undo these changes as recommended.
There was a problem hiding this comment.
This comment can be dropped.
Fishrock123
left a comment
Fishrock123
left a comment
There was a problem hiding this comment.
I seem to recall not having done this for some specific reason but now I'm not sure what it was if it was anything...
There was a problem hiding this comment.
Could you please name the wrapped version something else and update the call points respectively?
There was a problem hiding this comment.
Can you please look at it now? Thank you. I hope it was Ok to --amend this change?
There was a problem hiding this comment.
I hope it was Ok to --amend this change?
Yeah that's fine
cool88
force-pushed
the
master
branch
2 times, most recently
from
9549df3 to
ef63593
Compare
|
This LGTM if CI is green. Ping @cjihrig @Fishrock123 |
|
Landed in 6058c43, thanks for the PR! :) |
PR-URL: #12703 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: nodejs#12703 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: #12703 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: #12703 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
9 participants
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)