Conversation
kapouer
force-pushed
the
report
branch
2 times, most recently
from
e66fa9e to
0ce6fae
Compare
github-webhook.js
Outdated
| prefixStream(cp.stderr, 'stderr: ').pipe(logStream, { end: false }) | ||
| past.pipe(logStream, {end: false}) | ||
| } | ||
| if (rule.report) getStream(past).then(function(str) { |
There was a problem hiding this comment.
needs a { after the if
also, this should be exclusive to the logStream if above, we can't have the stream being piped into two places unless we introduce another utility that will allow it to be double-piped. It's either one or the other or something funky in between.
I also would rather not introduce get-stream, it's not a package I'm aware of and its usage here doesn't make me entirely comfortable -- there's no promises in this library yet so I'd like to avoid that for now, and there's no catch on this either. I'd rather use bl, which I'm obviously more comfortable with and it'll do the job just fine with a callback, like past.pipe(bl((data, err) => { err or data.toString() ... }
There was a problem hiding this comment.
Switched to bl (i like bl, i don't know why i used get-stream).
However i don't understand the comment piping in two places: i thought that was okay.
2 participants
Hi, this is something i just added. I thought it was cool enough to PR it !