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: 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

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

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

@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.

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

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

@Trott
Copy link
Member

Trott commented Apr 16, 2018

@lpinca
Copy link
Member

lpinca commented Apr 17, 2018

Landed in 31d6cec.

lpinca pushed a commit that referenced this pull request Apr 17, 2018
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 <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@lpinca lpinca closed this Apr 17, 2018
jasnell pushed a commit that referenced this pull request Apr 17, 2018
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 <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

10 participants