-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
util: deleted unreachable code from util.inspect #24187
Conversation
Just having a glimpse at it but I believe the line was reachable? |
@BridgeAR We did go through the code together, and it does not seem that way? |
@BridgeAR Could you take another look? |
Resume once again: https://ci.nodejs.org/job/node-test-commit/23173/ |
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.
Looks unreachable to me, as this code seems to be run only for strings with length ≤ 100 that contain special chars.
landed as 531d854 , thanks! |
PR-URL: #24187 Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
@ChALkeR you are correct. I forgot about the regular expression that makes sure that only strings reach that part that contain special characters. Thanks for pointing that out again. |
PR-URL: #24187 Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs#24187 Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: #24187 Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Contributed at NodeconfEU:
removed unreachable statement from util.inspect.