Skip to content

Adding NullLogging project#2

Closed
BrennanConroy wants to merge 2 commits intodevfrom
brecon/nulllogging
Closed

Adding NullLogging project#2
BrennanConroy wants to merge 2 commits intodevfrom
brecon/nulllogging

Conversation

@BrennanConroy
Copy link
Member

Choose a reason for hiding this comment

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

Run \\GarnetA\Share\KProjUpdateScript\UpdateKProjectFiles.ps1 on this (can just run it in global.json directory)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah! This gets me every time

@NTaylorMullen
Copy link

If you can add some really simple tests for these two classes real quickly so we have coverage for all of our common utilities. Can follow same format as the other test files. Will need to add a reference to Microsoft.Framework.Logging there (it will force build).

Choose a reason for hiding this comment

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

internal class

@NTaylorMullen
Copy link

@BrennanConroy
Copy link
Member Author

@NTaylorMullen

Choose a reason for hiding this comment

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

named parameters

@NTaylorMullen
Copy link

:shipit:

@BrennanConroy BrennanConroy deleted the brecon/nulllogging branch February 27, 2015 01:05
Copy link

Choose a reason for hiding this comment

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

why is this a public field instead of a property?

@dougbu
Copy link

dougbu commented Feb 27, 2015

⌚ minor comments mostly but would like to see a follow-up commit

@BrennanConroy
Copy link
Member Author

We are actually not doing this anymore, its moving to the Testing repo. I will look at the comments though

@dougbu
Copy link

dougbu commented Feb 27, 2015

ah, thought PR was closed because it went in

@NTaylorMullen
Copy link

It did, we reverted it.

natemcmaster pushed a commit that referenced this pull request Nov 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants