Skip to content
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

errors: replace .split() with .replace() #15545

Closed
wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Sep 21, 2017

Replace a somewhat idiosyncratic use of split() to remove a prefix
with replace(). (A case could be made for slice() as well but I
think this is more readable.)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

errors

Replace a somewhat idiosyncratic use of `split()` to remove a prefix
with `replace()`. (A case could be made for `slice()` as well but I
think this is more readable.)
@nodejs-github-bot nodejs-github-bot added the errors Issues and PRs related to JavaScript errors originated in Node.js core. label Sep 21, 2017
Copy link
Contributor

@Fishrock123 Fishrock123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm if ci looks good

@BridgeAR
Copy link
Member

@BridgeAR
Copy link
Member

Landed in 55d49eb

@BridgeAR BridgeAR closed this Sep 24, 2017
BridgeAR pushed a commit that referenced this pull request Sep 24, 2017
Replace a somewhat idiosyncratic use of `split()` to remove a prefix
with `replace()`. (A case could be made for `slice()` as well but I
think this is more readable.)

PR-URL: #15545
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@jasnell
Copy link
Member

jasnell commented Sep 25, 2017

This does not land cleanly in v8.x-staging, a backport would be needed. I believe it depends on a previous semver-major.

addaleax pushed a commit to addaleax/ayo that referenced this pull request Sep 30, 2017
Replace a somewhat idiosyncratic use of `split()` to remove a prefix
with `replace()`. (A case could be made for `slice()` as well but I
think this is more readable.)

PR-URL: nodejs/node#15545
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@Trott Trott deleted the errors-slice branch January 13, 2022 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
errors Issues and PRs related to JavaScript errors originated in Node.js core.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants