Allow setting MSBuild target via MSBuildSettings using a string

[augustoproiete]

Currently when calling the MSBuild alias with an MSBuildSettings, we need to set the target using the WithTarget extension method.

MSBuild("./my-app.sln", new MSBuildSettings
{
    Configuration = "Release",
    ToolVersion = MSBuildToolVersion.VS2019,
}.WithTarget("Build")); // <<<###

It would be nice if we could use a property, with a string, which would make it more natural when using properties for everything else:

MSBuild("./my-app.sln", new MSBuildSettings
{
    Target = "Build",  // <<<###
    Configuration = "Release",
    ToolVersion = MSBuildToolVersion.VS2019,
});

This property should also understand semicolons as separator for multiple targets and call Targets.Add accordingly.

MSBuild("./my-app.sln", new MSBuildSettings
{
    Target = "Clean;Build",  // <<<###
    Configuration = "Release",
    ToolVersion = MSBuildToolVersion.VS2019,
});

What version of Cake are you using?

1.0.0-rc0001

Would you be willing to send a PR?

Yes!

[ManasviGoyal]

can you help me get started? @augustoproiete

[augustoproiete]

@ManasviGoyal In MSBuildSettingsExtensions you can see how the WithTarget method works today, and in MSBuildSettings you can see that Targets is a read-only property of type ISet<string>.

The way I probably would approach this would be:

• Introduce a new MSBuildTarget type that implements ISet<string> to represent a collection of targets and maintain compatibility with existing Cake scripts that might inspect the Targets property
• Modify the Targets property of MSBuildSettings to use this new MSBuildTarget type
• Modify the Targets property of MSBuildSettings to be read/write (i.e. add a setter)
• Implement an implicit string operator on MSBuildTarget that can parse strings delimited by ;
• Write unit tests for all the above
• Write integration tests to make sure the conversion operator works

Hope that helps!

[MariaSolOs]

@augustoproiete I'm preparing a PR to address this, but when running the tests in Cake.Common.Tests I get a bunch of Code should not contain trailing whitespace errors :(
The weird thing is that all of these refer to lines with documentation comments... I would appreciate some insight on how to fix this! (see my implementation here)

[augustoproiete]

@MariaSolOs It's exactly what the error message says. Your documentation comments have trailing spaces on them that should be removed.

e.g.

[MariaSolOs]

@augustoproiete thank you!
What about Constructor summary documentation should begin with standard text? What's the standard text? I've been trying to find a reference for it in the contribution and documentation docs...

[augustoproiete]

@MariaSolOs https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1642.md

[MariaSolOs]

Thank you @augustoproiete! I apologize for all the newbie questions.

[MariaSolOs]

Okay now I'm not getting any errors in the code that I wrote, but when building I encounter the following:

I'm more than happy to submit a separate PR that addresses this (it seems like the tests are following a deprecated practice).

[augustoproiete]

Do these warnings also appear in the develop branch? If yes, then it's not something you have to worry about for this particular PR - though we'd welcome a separate PR in the future to improve that, of course.

If these warnings only appear in your new branch, it probably means that something in your code requires the test to be adapted - the test might not even be valid anymore as-is.

[MariaSolOs]

Right, that makes a lot of sense.
Thank you for your help!

[sbwaggoner]

Going to give this is a shot, assuming it's not yet been resolved.

[cake-build-bot]

🎉 This issue has been resolved in version v2.3.0 🎉

The release is available on:

• GitHub Release
• .NET Tool

Your GitReleaseManager bot 📦🚀