Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@jasnell
Copy link
Member

@jasnell jasnell commented Oct 26, 2017

Migrates most of CipherBase errors to use internal/errors.
There are still a handful remaining that need to be handled
separately

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

crypto

@jasnell jasnell added crypto Issues and PRs related to the crypto subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core. semver-major PRs that contain breaking changes and should be released in the next major version. labels Oct 26, 2017
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core. labels Oct 26, 2017
@jasnell
Copy link
Member Author

jasnell commented Oct 26, 2017

ping @nodejs/tsc

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

@jasnell
Copy link
Member Author

jasnell commented Oct 26, 2017

@jasnell
Copy link
Member Author

jasnell commented Oct 27, 2017

CI is good.

@jasnell jasnell force-pushed the migrate-cipherbase-internal-errors branch from d314326 to bd0ddad Compare October 27, 2017 15:28
Copy link
Member

Choose a reason for hiding this comment

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

Any reason this is inconsistent with #16529 (which return false on error?)

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch. no particular reason, just didn't remember ;-) I'll change this to false

@jasnell jasnell force-pushed the migrate-cipherbase-internal-errors branch from bd0ddad to 40d144e Compare October 29, 2017 23:38
Migrates most of CipherBase errors to use internal/errors.
There are still a handful remaining that need to be handled
separately
@jasnell jasnell force-pushed the migrate-cipherbase-internal-errors branch from 40d144e to acd2d94 Compare October 29, 2017 23:52
@jasnell
Copy link
Member Author

jasnell commented Oct 29, 2017

@jasnell
Copy link
Member Author

jasnell commented Oct 29, 2017

@joyeecheung ... PTAL

jasnell added a commit that referenced this pull request Oct 30, 2017
Migrates most of CipherBase errors to use internal/errors.
There are still a handful remaining that need to be handled
separately

PR-URL: #16527
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@jasnell
Copy link
Member Author

jasnell commented Oct 30, 2017

Landed in e567402

@jasnell jasnell closed this Oct 30, 2017
Qard pushed a commit to ayojs/ayo that referenced this pull request Nov 2, 2017
Migrates most of CipherBase errors to use internal/errors.
There are still a handful remaining that need to be handled
separately

PR-URL: nodejs/node#16527
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
Migrates most of CipherBase errors to use internal/errors.
There are still a handful remaining that need to be handled
separately

PR-URL: nodejs/node#16527
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core. 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.

6 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.