Skip to content
This repository was archived by the owner on Oct 17, 2018. It is now read-only.

Reacting to verbose rename #112

Merged
merged 1 commit into from
Dec 7, 2015
Merged

Reacting to verbose rename #112

merged 1 commit into from
Dec 7, 2015

Conversation

JunTaoLuo
Copy link
Contributor

Reacting to aspnet/Logging#314.
Tested locally.

@dnfclas
Copy link

dnfclas commented Dec 7, 2015

Hi @JunTaoLuo, 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;

{
return IsLogLevelEnabledCore(logger, LogLevel.Verbose);
Copy link
Member

Choose a reason for hiding this comment

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

Why is this not Debug? or an intentional behavior change?

Copy link
Member

Choose a reason for hiding this comment

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

Based on your other PR (logging) comment, this makes sense to me now...but the above LoggingExtensions.cs changes rename Verbose to Debug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an api change. There is already an IsDebugLevelEnabled function. The only thing that needs to change here is Verbose to Trace so every level has a corresponding helper function.

@JunTaoLuo
Copy link
Contributor Author

@kichalla I was wondering if you still had questions for this one?

@kichalla
Copy link
Member

kichalla commented Dec 7, 2015

:shipit:

@JunTaoLuo JunTaoLuo merged commit 79fca22 into dev Dec 7, 2015
@JunTaoLuo JunTaoLuo deleted the johluo/verbose-rename branch December 8, 2015 19:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants