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.

Sign up for GitHub

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

Jump to bottom

errors: alter ERR_INVALID_CURSOR_POS #19960

Closed
wants to merge 1 commit into from
Closed

errors: alter ERR_INVALID_CURSOR_POS #19960

wants to merge 1 commit into from

Conversation

davidmarkclements
Copy link
Member

@davidmarkclements davidmarkclements commented Apr 12, 2018 •
edited
Loading

changes the base instance for ERR_INVALID_CURSOR_POS
from Error to TypeError as a more accurate representation
of the error.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the errors Issues and PRs related to JavaScript errors originated in Node.js core. label Apr 12, 2018
@vsemozhetbyt
Copy link
Contributor

Additionally corrects the grammar of the error message.

Is this forgotten by chance?)

@vsemozhetbyt vsemozhetbyt added the readline Issues and PRs related to the built-in readline module. label Apr 12, 2018
@davidmarkclements
Copy link
Member Author

@vsemozhetbyt not relevant to this commit – thanks for spotting! removed

XadillaX
XadillaX approved these changes Apr 13, 2018
View reviewed changes
Copy link
Contributor

@XadillaX XadillaX left a comment

Choose a reason for hiding this comment

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

LGTM

@jasnell
Copy link
Member

jasnell commented Apr 14, 2018

CI: https://ci.nodejs.org/job/node-test-pull-request/14289/

@BridgeAR BridgeAR added semver-major PRs that contain breaking changes and should be released in the next major version. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Apr 15, 2018
@BridgeAR BridgeAR requested a review from a team April 15, 2018 22:15
@lpinca
Copy link
Member

lpinca commented Apr 16, 2018

This needs at least one more TSC approval before it can land.

mcollina
mcollina approved these changes Apr 16, 2018
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM

@lpinca
Copy link
Member

lpinca commented Apr 16, 2018

@davidmarkclements can you please rebase when you have some time? Thank you.

@lpinca
Copy link
Member

lpinca commented Apr 16, 2018

Would you be so kind to remove the merge commit?

@davidmarkclements
Copy link
Member Author

yes sorry - I used the github interface hoping it would rebase.. it doesn't – will fix

errors: alter ERR_INVALID_CURSOR_POS
3146154 
changes the base instance for ERR_INVALID_CURSOR_POS
from Error to TypeError as a more accurate representation
of the error.
@davidmarkclements
Copy link
Member Author

@lpinca - rebased

@lpinca
Copy link
Member

lpinca commented Apr 16, 2018

New CI: https://ci.nodejs.org/job/node-test-pull-request/14337/

@Trott
Copy link
Member

Trott commented Apr 16, 2018

Re-running Linux: https://ci.nodejs.org/job/node-test-commit-linux/18063/

@lpinca
Copy link
Member

lpinca commented Apr 17, 2018

Landed in 31d6cec.

lpinca pushed a commit that referenced this pull request Apr 17, 2018
@lpinca
errors: alter ERR_INVALID_CURSOR_POS
31d6cec 
Changes the base instance for ERR_INVALID_CURSOR_POS from Error to
TypeError as a more accurate representation of the error.

PR-URL: #19960
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Khaidi Chu <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
@lpinca lpinca closed this Apr 17, 2018
jasnell pushed a commit that referenced this pull request Apr 17, 2018
@jasnell
errors: alter ERR_INVALID_CURSOR_POS
5135e24 
Changes the base instance for ERR_INVALID_CURSOR_POS from Error to
TypeError as a more accurate representation of the error.

PR-URL: #19960
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Khaidi Chu <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trivikr trivikr trivikr approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

@XadillaX XadillaX XadillaX approved these changes

@lpinca lpinca lpinca approved these changes

@jasnell jasnell jasnell approved these changes

@mcollina mcollina mcollina approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. errors Issues and PRs related to JavaScript errors originated in Node.js core. readline Issues and PRs related to the built-in readline module. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@davidmarkclements @vsemozhetbyt @jasnell @lpinca @Trott @mcollina @XadillaX @BridgeAR @trivikr @nodejs-github-bot