grncdr / merge-stream

Merge multiple streams into one interleaved stream
MIT License
214 stars 16 forks source link

Pass an array as an argument #5

Closed cognitom closed 10 years ago

cognitom commented 10 years ago

To pass an array of streams to merge-stream, we have to write like below, now.

var gulp = require('gulp');
var merge = require('merge-stream');

gulp.task('css', function(){
  var tasks = [/* some tasks */];
  return merge.apply(null, tasks);
});

But it seems not intuitive for beginners. I'd like to change it little bit like below. How about?

return merge(tasks);
cognitom commented 10 years ago

I've just added a small test to this PR.

In another example, it would be nice to write return merge(tasks); instead of return es.concat.apply(null, tasks);.

gulp.task('scripts', function() { 
   var folders = getFolders(scriptsPath);
   var tasks = folders.map(function(folder) {
      return gulp.src(path.join(scriptsPath, folder, '/*.js'))
        .pipe(concat(folder + '.js'))
        .pipe(gulp.dest(scriptsPath))
        .pipe(uglify())
        .pipe(rename(folder + '.min.js'))
        .pipe(gulp.dest(scriptsPath));
   });
   return es.concat.apply(null, tasks);
});

https://github.com/gulpjs/gulp/blob/master/docs/recipes/running-task-steps-per-folder.md

grncdr commented 10 years ago

LGTM, but I'd prefer it if the array handling logic was moved into add so that the merge and add interfaces are consistent. Also, by putting it into add supporting arbitrarily nested arrays should be trivial.

cognitom commented 10 years ago

@grncdr Exactly! Thanks for your reviewing. I've just tried to move the logic into add :-)

grncdr commented 10 years ago

LGTM, thanks for the contributions :+1:

grncdr commented 10 years ago

published in 0.1.5 - I was having some sort of weird network glitch and 0.1.3 and 0.1.4 may not work...

cognitom commented 10 years ago

Thank you for your quick merge ;-)