handle orchestration aborted events #97
Conversation
There was a problem hiding this comment.
This is a great find, thanks for digging into the codebase and not only finding the issue, but also providing a PR to fix it!
I have a few comments below, mostly about code style changes, but also there appears to be an unused variable.
If you can get those changes in, I'll get this merged and published today.
index.js
Outdated
| @@ -67,8 +69,11 @@ function runSequence(gulp) { | |||
| } | |||
|
|
|||
| function finish(e) { | |||
| if (finished) return; | |||
There was a problem hiding this comment.
Finished appears to never be set.
There was a problem hiding this comment.
Also, please match the style of this codebase, no spaces between language keywords (if, catch, etc) and the parentheses.
index.js
Outdated
| @@ -96,6 +101,12 @@ function runSequence(gulp) { | |||
| } | |||
| } | |||
|
|
|||
| function onGulpError(e) { | |||
| if (e.message === 'orchestration aborted') { | |||
test/main.js
Outdated
| @@ -260,6 +260,40 @@ describe('runSequence', function() { | |||
|
|
|||
| called.should.eql(true); | |||
| }) | |||
|
|
|||
| it('should pass error if gulp execution halted in second execution', function(done) { | |||
| const stopTask = gulp.task('stopTask', function() { | |||
There was a problem hiding this comment.
To ensure backwards compatibility, since this codebase is older, let's leave this as a var. (I struggle with it too, since I've been writing const and let for quite a while now.)
test/main.js
Outdated
|
|
||
| it('should pass error if gulp execution halted in second execution', function(done) { | ||
| const stopTask = gulp.task('stopTask', function() { | ||
| if (stopTask.shouldStop) { |
There was a problem hiding this comment.
There's several spaces between the keywords and parentheses in here.
|
Thank you for the quick review! I'll update according your comments. |
|
Looks great, I'll get this published. |
|
OK, should be published as |
|
@OverZealous should be, 🤞 |
If running under a watcher, run-sequence will fail to trigger on subsequent calls to gulp.start when gulp.stop is called mid-sequence unless it handles the 'orchestration aborted' error.