-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
repl: handle buffered string logic on finish #24389
Conversation
@nodejs/repl |
Who do we need to cc to not forget including this fix in the security release that will be created soon? |
@vsemozhetbyt Oh if this gets included in that release, it will be great :) Thanks for the follow up. |
8e33b3a
to
2c179f3
Compare
@vsemozhetbyt You are super fast :) |
Bleh, some unrelated ARM failure. Thanks, landed in eb42c1e. (Hopefully I did that right... been an awfully long time since I landed anything...) |
Made an issue about the ARM failure (a crash): #24496 |
Landed on 11.x in c11d345 |
Thanks @Fishrock123 |
@Fishrock123 v11.x should only be updated when a release is merged. Can you force push to remove the commit? |
I fixed it, but had to temporarily unprotect the release branches to do so. |
@Fishrock123 No worries, btw :) I opened an issue in the Release repo to discuss a way to prevent this mistake in the future: nodejs/Release#392 |
Looks like `clearBufferedCommand` will be called on almost all flows. Hence history was broken. PR-URL: nodejs#24389 Fixes: nodejs#24385 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Fixes : #24385
Looks like
clearBufferedCommand
will be called on almost all flows. Hence history was broken. Sorry about that, but I take this as learning and see why history test cases didn't catch up this in first place. I will spend some time this weekend to add few more test cases on to repl history which caused this regression.Can some one ping repl team to make it land faster?
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes