Improved fluent API

[aarondandy]

The current fluent API does not match the style using in existing core plugins such as the Xunit plugin. The nuget plugin uses a form similar to WithSomeProperty(theValue) while Xunit uses SomeProperty(theValue). The fluent API within the NuGet plugin should do the same.

Also the API should be able to handle multiple files without needing to use lambdas. Currently the fluent API requires application with a lambda to apply to multiple commands. It would be much better if the API could operate on a list of commands.

nuget
.Pack(pack + ".csproj")
.Output(output)
.Properties(new { Configuration = "Release"})
.IncludeReferencedProjects()
.Verbosity(nugetVerbosity)
.Version(version + versionSuffix)))

[adamralph]

As discussed, the command methods, Restore(), Pack(), Push() should accept a params string[] and execute the exe once for each.

[aarondandy]

I can think of two ways to approach this.

One way is to create extension methods for the fluent APIs so that they can also be applied to IEnumerable<T> ... where T:Command , IEnumerable<Pack> , IEnumerable<Push> and IEnumerable<Restore> . This is the approach I am currently planning on implementing as it can just be applied on top of what exists to remove the lambdas.

I had another idea too. In this approach each child of Command would instead get a list of targets. For example public string NuSpecOrProject { get; set; } would change to ICollection<string> NuSpecsOrProjects { get {...} } . NuGetTask would still maintain a collection of commands but in most cases, even for two files there would only be one command, instead of two identical commands differing only by target. Because CreateCustomCommandLineArguments also appends the target it would need to return IEnumerable<IEnumerable<string>> which I think is getting a little too crazy for a tool that calls NuGet.exe . Also going down this route Command would probably need to be renamed CommandSet. Probably way over the top...

[adamralph]

Can the command objects not just hold an enumerable of targets? E.g.

public Restore Restore(params string[] solutionsOrPackagesConfigs)
{
    var restore = new Restore(solutionsOrPackagesConfigs);
    this.Add(restore);
    return restore;
}

[adamralph]

Actually, that's your second suggestion, right?

[aarondandy]

Yeah, with all the crazy complexity!

[adamralph]

Why do you think it's complex? the xunit plugin does the same - https://github.com/bau-build/bau/blob/dev/src/Bau.Xunit/Xunit.cs

[adamralph]

I realise that it means the model of holding a list of commands in the NuGetTask breaks down, but I think that's OK, you can just set one single command on it instead. It would be very unusual to want to set two different commands in the same task.

[aarondandy]

Yeah, that is very true. That is not the complexity I see though. Currently CreateCustomCommandLineArguments (https://github.com/bau-build/bau-nuget/blob/dev/src/Bau.NuGet/Pack.cs#L145) also adds the target file in as the first argument. Because of this I would recommend renaming that to just be CreateCustomCommandLineOptions and place more control over how the ProcessStartInfo in the hands of the children of Command

[aarondandy]

Either way I think we both agree that the list of commands in NuGetTask should be removed.

private readonly List<Command> commands = new List<Command>();

public IEnumerable<Command> Commands
{
    get { return this.commands.ToArray(); }
}

[aarondandy]

I am going to take a shot at option 2 (the xunit way) and if you don't like it, reject the PR when you wake up and I will branch at that point and we try something else. I think commits will explain my thoughts best with respect to this issue.

[adamralph]

BTW - here's another syntax idea:

.NuGetPack("pack").DependsOn("build", "clobber", "output").Do(pack=> pack
    .Project(pack + ".csproj")
    .Output(output)
    .Properties(new { Configuration = "Release"})
    .IncludeReferencedProjects()
    .Verbosity(nugetVerbosity)
    .Version(version + versionSuffix))

I.e. we have three separate task types, which can still inherit from a base. Then there's no need for a single task type to hold a command within it.

Perhaps you can think of a better name for the method which specifies the project or nuspec file, or perhaps .Project() and .Nuspec() can just be synonyms for the same method.

[aarondandy]

That is how I did it at first. I agree it should go back to that with the removal of Commands.

[aarondandy]

Also, I like .Target(pack + ".csproj") .

[adamralph]

I'm a little wary of 'target' since that's often used in other build systems, e.g. Make, MSBuild, Fake, to mean what we call 'task' in Bau.

[adamralph]

In fact, an early prototype of Bau used Target instead of Task.

Target gives me bad dreams about MSBuild XML and Task can get mixed up with System.Threading.Task.

I'm still not sure if that was the right decision or not...

[aarondandy]

I share that feeling but unless somebody makes something like static void Target(this object stuff){ /*crazy stuff*/ } we would be technically fine. From a nomenclature point of view it is definitely an abuse of the word target though as it is a build system.

I think Task is OK as it does not have MSBuild baggage and it is only applied to Bau types.

[adamralph]

BTW - if you want to be consistent with the xunit plugin:

.Use(string exe)    // for overriding the exe path
.In(string workingDirectory)
.With(string args) // future proofing for new args, until we add them to the API

[adamralph]

Actually, With(string args) was probably a mistake with the xunit plugin, it probably should have been With(params string[] args), which is what the exec plugin has.

[aarondandy]

What about

.NuGetPack("pack").DependsOn("build", "clobber", "output").Do(pack=> pack
    .Files(pack + ".csproj")
    ....

[adamralph]

Yep, that looks good. On 1 Dec 2014 01:41, "Aaron Dandy" notifications@github.com wrote:

What about

.NuGetPack("pack").DependsOn("build", "clobber", "output").Do(pack=> pack .Files(pack + ".csproj") ....

— Reply to this email directly or view it on GitHub https://github.com/bau-build/bau-nuget/issues/31#issuecomment-65008216.

[adamralph]

Come to think of it, there should probably be an abstract base in core which gives you Using, In and With. On 1 Dec 2014 01:57, "Adam Ralph" adam@adamralph.com wrote:

Yep, that looks good. On 1 Dec 2014 01:41, "Aaron Dandy" notifications@github.com wrote:

What about

.NuGetPack("pack").DependsOn("build", "clobber", "output").Do(pack=> pack .Files(pack + ".csproj") ....

— Reply to this email directly or view it on GitHub https://github.com/bau-build/bau-nuget/issues/31#issuecomment-65008216.