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

Ensure that bundling errors make are surfaced. Fixes gh-441 #444

Merged
merged 1 commit into from
Nov 17, 2015

Conversation

rwaldron
Copy link
Contributor

No description provided.

@@ -436,7 +436,10 @@ actions.tarBundle = function(opts) {
})
.bundle(function(error, results) {
if (error) {
return reject(error);
return reject({
Copy link
Contributor

Choose a reason for hiding this comment

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

A bunch of other places in the code return a string for a warning and an actual Error for a more stern error output. Why not return an Error here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because those are all taking the easy path that automatically logs with logs.warn, not logs.err. I explicitly wanted logs.err and this was the only way to force it to happen, because:

t2-cli/bin/tessel-2.js

Lines 290 to 307 in e7299f0

module.exports.closeFailedCommand = function(opts, err) {
if (!err) {
err = opts;
opts = {};
}
if (err instanceof Error) {
throw err;
} else {
// Print a stern warning by default
opts.type = opts.type || 'warn';
logs[opts.type](err);
}
if (opts.code === undefined) {
opts.code = 1;
}
process.exit(opts.code);
};

@HipsterBrown
Copy link
Contributor

This makes browserify even more helpful for warning developers about syntax errors before pushing to Tessel. Looks good to me. 👍

rwaldron added a commit that referenced this pull request Nov 17, 2015
Ensure that bundling errors make are surfaced. Fixes gh-441
@rwaldron rwaldron merged commit 0726002 into tessel:master Nov 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants