Skip to content

feat: adding error codes to ShellJS #394

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
Merged
ariporad merged 2 commits into from
Mar 17, 2016
Merged

feat: adding error codes to ShellJS #394

ariporad merged 2 commits into from
Mar 17, 2016

Conversation

@nfischer
Copy link
Member

@nfischer nfischer commented Mar 16, 2016

This is a redo of #269.

This adds error codes as a .code attribute to ShellStrings, but doesn't implement the errorCode() function.

This also does not switch the implementation of the error() function (that should be in another PR, IMO).

Tests will be added soon.

@nfischer nfischer added this to the v0.7.0 milestone Mar 16, 2016
@nfischer nfischer mentioned this pull request Mar 16, 2016
Closed
@nfischer
Copy link
Member Author

nfischer commented Mar 16, 2016

@ariporad @levithomason This should be ready for review.

This adds the .code attribute to everything.

If we like this implementation and merge this, we can open a new PR to change error() to return the code instead of the last command's error string. I didn't include it in this PR because error codes themselves shouldn't be a breaking change, and it's easy enough to swap that implementation in a separate PR.

I left this as 2 commits, since the first one adds error codes, and the second one refactors everything to return a ShellString (which gives every command the .code and .stderr attributes). Feel free to squash if you prefer.

ariporad
ariporad reviewed Mar 16, 2016
View reviewed changes
test/pipe.js
//

// no piped methods for commands that don't return anything
assert.throws(function() {
Copy link
Contributor

@ariporad ariporad Mar 16, 2016

Choose a reason for hiding this comment

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

Why did this get removed?

Copy link
Member Author

@nfischer nfischer Mar 16, 2016

Choose a reason for hiding this comment

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

Almost every command returns a ShellString now. So the number of "commands that don't return anything" has shrunk to almost zero.

Copy link
Contributor

@ariporad ariporad Mar 17, 2016

Choose a reason for hiding this comment

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

OK.

@ariporad
Copy link
Contributor

ariporad commented Mar 17, 2016

LGTM!

ariporad added a commit that referenced this pull request Mar 17, 2016
@ariporad
Merge pull request #394 from shelljs/feat-error-codes
f006fcd 
feat: adding error codes to ShellJS
@ariporad ariporad merged commit f006fcd into master Mar 17, 2016
@ariporad ariporad deleted the feat-error-codes branch March 17, 2016 02:29
This was referenced Mar 17, 2016
Closed
Closed
@nfischer nfischer mentioned this pull request Mar 28, 2016
Closed
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

feature medium priority

Projects

None yet

Milestone

v0.7.0

Development

Successfully merging this pull request may close these issues.

3 participants

@nfischer @ariporad