Should tasks inherit the branch flag

[phated]

@erikkemperman
I am taking a look at implementing gulpjs/gulp#1411 (comment) and gulpjs/gulp#1330 through the use of the branch flag you added to the metadata (awesome idea, btw). However, when we do something like the following, that flag isn't attached to the metadata for the new task.

gulp.task('default', gulp.series(...));

Do you think that the task should inherit the branch flag if we already have metadata and the metadata tells us it is a branch?

[phated]

Implementation at 3536ee7 - needs tests

[erikkemperman]

@phated So, the idea is to use the branch property to make decisions about error propagation and log level, have I got that right?

I added the flag to be selective about when to do nodes.push in set-task.js -- before it'd always do that if metadata already existed for the fn at hand, the assumption apparently being that this would only happen for series/parallel constructions, which was no longer true with aliases.

I don't see how it would functionally wrong to have tasks "inherit" this property from their function (although I might not call it that)... But I didn't have a lot of time playing around with it.

With the gulp-cli branch branches-debug-log, which I assume is the intended corresponding change to passing the branch flag along in createExtensions, the output hides too much in my opinion. In stead of just suppressing <series> and <parallel> lines it also suppresses the parent task.

Maybe it's just me but if I run gulp foo I expect to see Starting foo and Finished foo in the output, without having to do -LLLL. So I guess that might be an argument against inheriting the branch property in this way.

I'm not too familiar with error propagation, but wouldn't it be pretty easy to just mark an error as having been logged, and not printing it again in that case? That way you could separate error propagation from error logging.

[phated]

@erikkemperman Yeah, I agree that running gulp foo should show "Starting foo" and "Finished foo" but if the branch flag doesn't transfer to the named task, some error logging doesn't get hidden (errors from series/parallel are hidden) with this change. Maybe that's okay. I don't know of a good way to handle the error propagation

[erikkemperman]

@phated Not at a computer atm, but just a thought: maybe pass along meta.tree.type as well? I.e., the combination of branch and type might allow you to print or suppress in the appropriate circumstances?

[erikkemperman]

Or, now looking at your gulp-cli change, maybe only use the branch property for logging errors, leaving the start/finish stuff same as before..

[phated]

@erikkemperman I'm going to close this. I've come up with a better way to handle the error propagation and I'm only going to filter the <series> and <parallel> logging based on the branch flag.

[erikkemperman]

@phated sounds good to me!