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

GH702 GH473: Added logger switch to MSBuild runner #1198

Merged
merged 1 commit into from
Sep 4, 2016

Conversation

cpx86
Copy link
Contributor

@cpx86 cpx86 commented Aug 29, 2016

Relates to #702 and #473.

The gist of it is fairly simple. I've added an extension method for adding a logger switch to the MSBuildSettings class, similar to how property switches are added, like so:

public static MSBuildSettings WithLogger(this MSBuildSettings settings, string loggerAssembly, string loggerClass = null, string loggerParameters = null)

Calling that method should append an argument to MSBuild, like so:
/logger:loggerClass,loggerAssembly;loggerParameters

In addition to this, I added a MSBuildLogger class to hold the above assembly, class and parameters values, and to the MSBuildSettings class I added a collection property to hold instances of those.

@dnfclas
Copy link

dnfclas commented Aug 29, 2016

Hi @cpx, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

@dnfclas
Copy link

dnfclas commented Aug 29, 2016

@cpx, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

@gep13 gep13 changed the title Added logger switch to MSBuild runner GH702 GH473: Added logger switch to MSBuild runner Aug 29, 2016
@patriksvensson
Copy link
Member

@cpx Looks good to me! 👍 Great job! I left some (minor) comments for you.

@cpx86
Copy link
Contributor Author

cpx86 commented Sep 3, 2016

@patriksvensson Done!


// Then
Assert.Equal(2, settings.Loggers.Count);
Assert.Equal("LoggerAssembly1", settings.Loggers[0].Assembly);
Copy link
Member

Choose a reason for hiding this comment

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

ICollection doesn't support indexing, LINQ could be one option istead
I.e.

var first = settings.Loggers.First();
var second = settings.Loggers.Skip(1).First();

and use those variables for the asserts.

@cpx86
Copy link
Contributor Author

cpx86 commented Sep 3, 2016

Oops, was a bit quick to push there :) Now it should work.

@devlead
Copy link
Member

devlead commented Sep 3, 2016

@cpx excellent, could you squash into 1 commit and rebase against latest develop, and ping it's when done.

@cpx86
Copy link
Contributor Author

cpx86 commented Sep 3, 2016

@devlead Done.

@cpx86
Copy link
Contributor Author

cpx86 commented Sep 4, 2016

Hmm, any idea why the OSX build failed? Is it due to changes in this PR or something else?

@patriksvensson patriksvensson merged commit 0542edc into cake-build:develop Sep 4, 2016
@patriksvensson
Copy link
Member

@cpx Merged 👍 Thanks for this contribution! 😄

@cpx86 cpx86 deleted the feature/GH-702 branch September 10, 2016 16:12
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