Uh oh!
There was an error while loading. Please reload this page.
Breaking: use VError base signatures. migrate jscs to eslint. - #86
Conversation
coveralls
commented
Feb 18, 2018
| @@ -1,14 +1,16 @@ | |||
| { | |||
| // eslint: recommended automatically enables most/all rules from the | |||
There was a problem hiding this comment.
The commit message should reflect that the tooling was updated in this PR.
There was a problem hiding this comment.
👍 done, JSCS was freezing/failing for unknown reasons, figured no better time to swap it out.
| The constructors can be used to new up Error objects with default status codes | ||
| set. | ||
| This module ships with a set of constructors that can be used to new up Error |
There was a problem hiding this comment.
new Error, can rephrase if confusing
| options: options | ||
| }; | ||
| // reconstruct verror ctor options from the cleaned up options | ||
| verrorArgs = [ verrorOpts ].concat(_.tail(args)); |
There was a problem hiding this comment.
Why is verrorArgs not just verrorOpts? Doesn't adding args back to verrorOpts result in the overlap of the sets being included in the array twice?
There was a problem hiding this comment.
VError has various signatures, the one we care about is new VError({}, 'sprintf', 'sprintf args' ...)
restify-errors options can be appended to the first arg - so what we do here is strip out restify-error specific options from opts, rename the options leftover as verrorOpts, then concat with everything but the first argument of raw args (the first argument being itself).
| // code property doesn't have 'Error' in it. remove it. | ||
| var defaultCode = name.replace(new RegExp('[Ee]rror$'), ''); | ||
| var d = _.assign({}, { |
There was a problem hiding this comment.
I know it's not in the linter but I prefer avoiding one letter variables as it's hard to know what they mean and also hard to search and replace them.
DonutEspresso
commented
Feb 21, 2018
Comments addressed, once more? |
DonutEspresso
commented
Feb 21, 2018
Thanks all. Excited to get this in, this has been a long time coming. |
Stop ignoring package-lock.json and commit it so `npm ci` (used in the new GitHub Actions workflows) has a lockfile to install from. It was previously untracked/ignored (since #86), which made sense before npm ci existed, but now blocks reproducible CI installs. This has no effect on the published package, since "files": ["lib"] already excludes the lockfile from the npm tarball. Co-authored-by: Cursor <cursoragent@cursor.com>
Fixes#76, #77.
This jettisons any custom restify-error constructor APIs in favor of using VError's constructor APIs. This also eliminates the custom
.contextproperty in favor of using VError'sinfoconstruct. Acontextgetter remains on all restify-error base classes to facilitate with migration. More details in README, but this should essentially make restify-errors completely compatible with the VError static methods.