DependsOnAsync, AddTasksAsync and DoAsync naming

[mzorec]

Suggestion from @huanlin :

While fixing this bug, one idea popped up:

The tasks configured with DependsOnAsync method are actually executed with Task.Run, which is arguably not asynchronous calls. I mean the following code in TaskBase.cs:

protected virtual async Task<TResult> DoExecuteAsync(ITaskContextInternal context)
{
    return await Task.Run(() => DoExecute(context));
}

Having said that, I don't have better ideas to run those "Async" tasks simply because we don't know what those user-defined tasks really do (could be CPU-bound or I/O bound).

I'm just feel that DependsOnAsync might mislead people, make them think that those tasks are executed with C# asynchronous calls.

So one possible change is to rename DependsOnAsync to something else, maybe DependsOnParallel or DependsOnConcurrently ?

I may be wrong or missed something though. So this is just a note. No further action requested. (discussions are welcomed)

[mzorec]

I'm just feel that DependsOnAsync might mislead people, make them think that those tasks are executed with C# asynchronous calls.

We actually had this discussion before with my brother. The problem is that it might be parallel or it might be async. You are looking at the implementation in the TaskBase which is misleading because it's a base class and DoExecuteAsync can be overriden..

For example take implementation of this simple task:

        protected override async Task<int> DoExecuteAsync(ITaskContextInternal context)
        {
            await Task.Delay(_delay);

            return 0;
        }

This task is executed with C# asynchronus call.

So DependsOnAsync naming is so so. Probably better name would be as you suggested because currently more tasks are really not made with asynchronus calls but in some scenarios it might also be misleading. But there is another problem with DoAsync everything with it will almost surely gona be made with asynchronus calls.

    protected override void ConfigureTargets(ITaskContext context)
        {
            context.CreateTarget("Sample").DoAsync(Sample);
        }

        private async Task Sample(ITaskContext context)
        {
            var httpClient = new HttpClient();

           await httpClient.GetAsync("");
        } 

So we should change DependsOnAsync to DependsOnConcurrent but leave DoAsync? This might also be confusing.

I am affraid that here we will not find the "100% right" solution but we can definetly change if there are some really good arguments why should we change or If more people votes for any of the proposed solutions

Edit: Oh yea and if we take this scenario:

  context.CreateTarget("Rebuild")
            .SetAsDefault()
            .AddTask(x => x.Do(t => { Console.WriteLine("running Rebuild."); }))
            .DependsOnAsync(doExample, doExample2)
            .DependsOn(test); 

This is not parallel anymore :)

[huanlin]

With your examples, now I understand more about it, and I don't have better ideas for now.
Maybe just put some words in documents to avoid misunderstanding.... wait, I do see "parallel" mentioned in the document: Asynchronus or parallel execution of tasks, customCode and dependencies.

So yeah, maybe we do nothing for now, wait and see if there are more ideas coming up later.

[mzorec]

Maybe also some better explanation on the method documentation (summary).

[mzorec]

If you think or anyone else how could this be explained better in the documentation Asynchronus or parallel execution of tasks, customCode and dependencies or you fell some parts are missing. PR would be more than welcome. Also if anyone have an idea how could we explain this briefly in method documentation please provide a PR :)