Update docs

[bnjmnt4n]

The docs mention that settle{Series,Parallel} can be accessed on the bach object, but AFAIK they are only accessible via require("bach/settle"). Perhaps you should update the docs about this.

[phated]

Sorry about that. The docs were the original thought about making everything accessible on the bach namespace, but then I added timing and thought this could be better. I am still not completely sold on this approach. Thoughts?

[pkozlowski-opensource]

@phated is this somehow related to you not being happy about async-time in this comment?

I'm just going over all the Gulp4-related repos and trying to piece all the elements together. But during this process I'm asking myself the following question: should we time all the functions or just tasks? In other words: what is the relation of bach (which, if I understand correctly, is primarily concerned with functions composition / orchestration) and async-time. Maybe timing should just come to the picture on the higher (task) level?

When it comes to settling parallel / series functions - I would be curious to know what were real-life scenarios where you guys thought of introducing them? I was naively thinking that we want to have the fail-fast semantics for build steps.

Sorry if I'm asking stupid questions here - still wrapping my head over various pieces of the Gulp4 puzzle...

[phated]

@pkozlowski-opensource you are correct, I am not happen with the latest bach API due to async-time needing bach to be an event emitter. The need for timing should only be needed at the task level, but then it would have to happen inside parallel and series inside orchestrator, which seems weird. I actually was heading down that path before async-time. I might return to doing that to keep bach's API as simple as possible.

The scenario for settling is partial builds. In gulp, people have requested that if one task fails, not to fail the other tasks. Settling allows all tasks to complete (series or parallel) before reporting the errors that occurred. Also, it is a pattern that is seen in promise libraries but not in async libraries I have looked at. By exposing it in bach, gulp/orchestrator can utilize a settle function if a partial build option is set.

The questions aren't stupid and I am glad someone cares about the design decisions here, as most people have ignored the work towards gulp4.

[bnjmnt4n]

IMO a possible solution is to make bach always an EventEmitter, by shifting all the functions to the main index.js and always timing everything.

[phated]

@D10 we won't be timing everything unless it is unobtrusive. Timing currently adds 1 or 2 closures around the user supplied function to do the timing.

[pkozlowski-opensource]

FWIW, the more I'm looking into Gulp4 related repos the more I in favour of moving all the timing-related responsibilities out of bach. I kind of like the idea of bach being just concerned with sequences / parallel execution of "async" functions.

@phated I would be interested in hearing more about why putting those responsibilities on the orchestrator level seems weird to you. Is it the fact that we would have to wrap timing functionality around each argument passed to series / parallel? What would be a practical concern here?

[phated]

@pkozlowski-opensource I actually remembered why we can't do it in the orchestrator tier, and that is because you don't have access to the completion of a single task. Since a task can end in a variety of ways and that is handled by async-done, there is no easy way to wrap a task function to know when it finished in any way. My initial implementation passed a callback function in, but tasks that returned streams were never finished since they didn't call that timing callback.

[phated]

@pkozlowski-opensource i am 100% open to having orchestrator be the timing tier and simplifying bach once again, but I am at a loss for how to do it. All thoughts welcome.

[pkozlowski-opensource]

@phated I'm still (very) new to those things but assuming that we want to remove all the timing thing from bach and move it to the orchestrator level, couldn't we wrap tasks / private functions:

• tasks functions: when registering tasks
• "private functions" (any async functions really): when passing those functions to orchestrator.series / parallel calls

I was experimenting with the code in this commit: pkozlowski-opensource/orchestrator@7ad302f and it seems to work but I'm sure I'm missing 10 000 different things here... The one thing that is missing for sure is the ability to specify a name to async-time instead of always letting it figure out the name from the function name (so we can provide a task name for an anonymous task function).

Anyway, sharing what I had on my mind - even if stupid maybe it will trigger some discussion / other ideas...

[phated]

the problem I have with wrapping with async-time inside orchestrator is that async-time wraps async-done and bach uses async-done, so we have at least 3 layers of closures to get timing and those create extra domains, etc.

[pkozlowski-opensource]

@phated right, those multiple layers of wrapping are valid concern. I just wonder if this isn't natural conflict between trying to keep all the elements of the stack (bach, async-done, async-time etc.) generic and re-usable with any async functions.

I mean, if we would do the async-time wrapping on the orchestrator level we would know that every function that goes to layers below (bach) is callback-based. So we could collapse all those wrapping layers by ... not using async-done in bach, but it would obviously make it (almost) a proxy to async (and limit it general aspect) :-/

At the moment it seems to me like there is a tension between building on top of generic building blocks vs. having minimal code / operations - which is not uncommon in programming :-)

I'm not very familiar with domain and still quite new to this whole node-fu so not sure what are practical implications of having those 3 layers of closures / domains in the context of gulp / orchestrator?

[phated]

@pkozlowski-opensource I think I solved the abstraction problem. The thing that was the most restricting was using async for its map and mapSeries functions. I just wrote the basis for async.map/mapSeries-like functions that allows for extension points. See https://github.com/phated/now-and-later. This will allow us to inject code at before, after, and on error of each function executed by async-done. I think this will be the basis for bach and the new task system. I would love your thoughts on it.

[pkozlowski-opensource]

@phated cool! I gave it a try and the whole thing is easy to embed in the orchestrator (pkozlowski-opensource/orchestrator@3a431a0). Also adding timing is easy: https://github.com/pkozlowski-opensource/orchestrator/blob/time_now_and_later/index.js so all looks good here. I've also combined everything into a working POC: https://github.com/pkozlowski-opensource/gulp4-poc/tree/now_and_later_time

Now, while playing with those various pieces I've noticed a couple of things.

Method signatures

• if we want to tackle streams draining on the orchestrator level the signature of the after function should change from after(fn, key) to something like after(fn, key, result) or after(result, fn, key) , same for errors
• not sure how I feel about ability to pass both array and object arguments. Object args can help with naming but arrays provide stable order... Need to think about it some more.

Orchestrator's repository cleanup

• there is probably no reason for the Orchestrator's repository to deal with tasks' timing' right? This should be responsibility of the orchestrator itself and any repository should benefit from timing events. Or am I getting it all wrong?
• there is some code duplication (names normalization) between repository and the orchestrator itself

Naming

• one would have to use named functions for tasks consistently to get meaningful timing events in series - this might be a bit of the problem for people used to registering anonymous functions today;
• the issue is even more visible for tasks registered like this: gulp.task('default', ['foo', 'bar']), provided that we want to support this syntax for gulp4 (IMO it would ease the migration); AH! Unless you were planning to use object notation to propagate names (this will work ok for parallel, should work OK for series as well, no?)
• functions returned from series / parallel should be named - once again, this is to have some names in the start / end callback

After looking into object-notation some more this might be remedy for naming problems, need to think about it some more.

[pkozlowski-opensource]

@phated in short: this all looks very promising: simple and I can see it working. Still there are couple of things to clarify:

• naming tasks / functions
• draining streams
• error reporting from orchestrator (task not found etc.)
  as those things will (most probably) affect various APIs.

I can start opening individual issues for various repos / tackling various items as soon as we agree on the above. I would really love to see the new task system in place sooner than later so willing to put effort here.

[phated]

I just updated now-and-later to be a little more streamlined and configurable. I want it to exist as the layer underneath bach. bach will be the composition layer that returns a closure, which reduces overhead for anyone wanting to use now-and-later in place of async.map/mapSeries (since the API can be used in the exact same way now).

[phated]

The other thing I added was a create extension point that returns a value to be passed to each other extension point. This brings the API very, very close to that of async-listener and it makes a lot of sense because we need to get the task name and generate a unique ID for gulp.

[phated]

I might remove the dependency on async-done and move that into bach. Thoughts?

[phated]

Good news! bach is basically back to the documented API, with an added feature of optionally taking an object of extension points as the last parameter to each method. This will allow us to do timing in orchestrator while still keeping bach a useful library outside of it.