Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NOT REQUIRED: Adding a sample with multiple arguments #1043

Closed
wants to merge 3 commits into from

Conversation

robertmuehsig
Copy link
Contributor

This might be obvious, but because of the nature of a cake script (currently no intellisense) I needed some minutes to understand what the ArgumentCustomization is doing and how I can append two or more arguments.

This might be obvious, but because of the nature of a cake script (currently no intellisense) I needed some minutes to understand what the ArgumentCustomization is doing and how I can append two or more arguments.
@dnfclas
Copy link

dnfclas commented Jul 11, 2016

Hi @robertmuehsig, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

@@ -39,6 +39,10 @@ public class ToolSettings
/// new NuGetSourcesSettings { UserName = "user", Password = "incorrect",
/// ArgumentCustomization = args=>args.Append("-StorePasswordInClearText")
/// });
/// // or with multiple arguments:
/// MSTest(pathPattern, new MSTestSettings() { ArgumentCustomization = args => { args.Append("/detail:errormessage");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robertmuehsig thanks for adding this, I think it makes a lot of sense. One comment though...

Can you change the > to be > as per the first example, otherwise bad things happen 😄

@devlead
Copy link
Member

devlead commented Jul 11, 2016

@robertmuehsig this can actually be simplified as you'll get a builder back you can just

MSTest(pathPattern,
    new MSTestSettings {
        ArgumentCustomization = args => args.Append("/detail:errormessage")
            .args.Append("/resultsfileTestResults.trx")});

Written on phone so might not 100% correct & formatted, but hopefully you get the gist of it 😉

@gep13
Copy link
Member

gep13 commented Jul 11, 2016

@robertmuehsig looks like there are a couple StyleCop errors related to this PR. You can see the build log here:

https://ci.appveyor.com/project/cakebuild/cake/build/0.14.0-PullRequest.1043+55.build.2315

Any chance you can fix them up? Thanks!

@devlead
Copy link
Member

devlead commented Jul 11, 2016

@robertmuehsig
Copy link
Contributor Author

Mhh... I will take a look at it later and resolve the StyleCop issues and provide multiple examples. Thanks guys! :)

@@ -39,6 +39,9 @@ public class ToolSettings
/// new NuGetSourcesSettings { UserName = "user", Password = "incorrect",
/// ArgumentCustomization = args=>args.Append("-StorePasswordInClearText")
/// });
/// // or with multiple arguments:
/// MSTest(pathPattern, new MSTestSettings() { ArgumentCustomization = args => args.Append("/detail:errormessage")
/// .Append("/resultsfile:TestResults.trx") });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use spaces not tabs
image
so that's likely the stylecop issue.

@robertmuehsig
Copy link
Contributor Author

Close because I created a new PR #1047

@gep13
Copy link
Member

gep13 commented Jul 11, 2016

@robertmuehsig said...
Close because I created a new PR #1047

Was there something wrong with this one?

@robertmuehsig
Copy link
Contributor Author

Well - TBH - I started this PR via the GitHub Web UI (thats why the tabs instead of the spaces) and decided to start over on my workmachine because I thought it would be easier to not fightling against the git commands.
Mhh... this idea was not the brightest, because even with the GitHub4Win Client I could made those changes... well... I'm still a novice with Git - my bad! Hope it is ok for you 😃

@gep13
Copy link
Member

gep13 commented Jul 12, 2016

@robertmuehsig said...
Hope it is ok for you 😃

Oh, absolutely, I just wanted to make sure that everything was ok. Git, GitHub and Pull Requests are very clever once you get your head around them, Even if you think things are very broken, there are likely things that can be done to correct them, so opening a new PR is not always necessary. In this instance, creating a new PR is a perfectly valid approach. Like I say, just wanted to make sure that everything was ok 😄

@gep13 gep13 changed the title Adding a sample with multiple arguments NOT REQUIRED: Adding a sample with multiple arguments Jul 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants