Skip to content

Conversation

@floatdrop
Copy link
Contributor

Only after all tasks are done. Closes #88

Only after all tasks are done. Closes #88
@yocontra
Copy link
Member

yocontra commented Jan 4, 2014

Isn't this already happening? If a task errors and nobody is handling it then it gets thrown and node will exit with 1 right

@floatdrop
Copy link
Contributor Author

No, it doesn't. :(

yocontra added a commit that referenced this pull request Jan 5, 2014
Exit with code 1 when got errored tasks
@yocontra yocontra merged commit 4c1300d into gulpjs:master Jan 5, 2014
@garthk
Copy link

garthk commented May 12, 2014

Regressed. /ping @phated to be tackled during rewrite.

@garthk
Copy link

garthk commented May 12, 2014

Ugly workaround:

function fixExitStatus(gulp) {
    var exitStatus = 0;

    gulp.on('err', function () {
        exitStatus = 1;
    });

    process.on('exit', function (status) {
        if (status < exitStatus) {
            process.exit(exitStatus);
        }
    });
}

Usage: fixExitStatus(gulp);

@robrich
Copy link
Contributor

robrich commented May 12, 2014

This is a side effect of
https://github.com/gulpjs/gulp/blob/master/bin/gulp.js#L102 - task err are
swallowed.

@robrich
Copy link
Contributor

robrich commented May 12, 2014

I grant we passed on https://github.com/orchestrator/orchestrator/tree/develop, but this is solved there, and https://github.com/orchestrator/orchestrator/tree/blaine isn't done yet. For most users, they'll never see gulp.run or gulp.start or runParallel or series, so swapping this in now and swapping bach in later shouldn't be too traumatic.

@yocontra
Copy link
Member

@robrich Can you provide a list of what APIs changed between the version gulp is on now vs. your current? If it doesn't break anything I'm all for using it as a stopgap until the next one

@phated
Copy link
Member

phated commented May 13, 2014

@contra There were a lot of breaking changes in @robrich's develop branch, that's why I thought we would wait. I started adding more tests for orchestrator this weekend.

@robrich
Copy link
Contributor

robrich commented May 14, 2014

@contra: the develop branch's API is completely additive with two exceptions: each orchestration is now separate so robrich/orchestrator#15 now works the way you'd expect: it calls the callback after each orchestrator.run instead of after everything. Most argue this is a fix. The other is sync tasks are prohibited (which we could easily revert by changing https://github.com/orchestrator/orchestrator/blob/develop/lib/runOne/runTask.js#L58)

Here's other interesting facts comparing orchestrator's develop with master:

  • The sample gulpfile.js on https://github.com/gulpjs/gulp runs exactly as is -- no code changes necessary, no user-facing differences at all. I suspect most users' experiences will be similar. (Unfortunately, I'm sure we'll only hear about the cases that aren't. :D)
  • The changes necessary to make orchestrator changes imperceptible to gulp users are in https://github.com/robrich/gulp/tree/develop (which given how long they've sat should be rebased) and include: 1. shim gulp.start to orchestrator.runParallel, 2. shim gulp.onAll to EventEmitter2.onAny (or abandon it -- did anyone ever use it?) 3. shim or revert event names from old (task_start, task_stop, and task_err) to new (taskStart, taskEnd, and taskError) or just revert the name changes.
  • There are additive changes to the API which we could choose to document or ignore. Though they enable some interesting workflows, 90% of users probably could get by without knowing they're there: 1. parallel, series, and their counterparts runParallel and runSeries (which just call parallel/series then run) allow for easier composition of task workflows, 2. the task dependency array can contain strings and/or objects, specifying additional properties bound to this during task execution, 3. hasTask and taskNames functions make gulp -t easier, 4. an optional {continueOnError:true} flag passed to orchestrator.run allows watch tasks to not fail the build when a task fails.

@phated
Copy link
Member

phated commented May 14, 2014

@robrich based on the way gulp is working, the orchestrator change would be breaking. The other things that you mention could be reverted, should be as those are available to end users since gulp is an instance of orchestrator.

@yocontra
Copy link
Member

Fixed in 3.8.4

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.

gulp/gulp.cli is exits with code 0 if managed to start

5 participants