LogLevel.None

[WilBloodworth]

Please either change LogLevel.None's value to 0 or provide a different log level that has a value of 0.

Right now, there isn't an entry in this enumeration with the value of 0. I consider this an error even though it is allowed (i.e. compiles). For example, if you take, "default(LogLevel)" what does it return?... 0. But 0 isn't a valid value for LogLevel.

Please simply change None to be 0 and all will be right with the world. I honestly don't care which way you change the ordering (low to high), (high to low)... as long as this enumeration ends up with an entry with a 0 value.

SIDE BAR: You could make it a 'flags' enumeration with values having the power of 2... that way you COULD mix and match the levels. Not sure who would do that... but you could.

Thank you!

[benaadams]

SIDE BAR: You could make it a 'flags' enumeration with values having the power of 2... that way you COULD mix and match the levels. Not sure who would do that... but you could.

Might work for listeners/loggers that want a couple levels but not others

[muratg]

@lodejard How about starting with -2 for Trace, and having 0 as Information?

[muratg]

@WilBloodworth using default on an enum is not a common case, AFAIK. What's the use case where you would do default(LogLevel)?

[muratg]

We had a discussion on this with @glennc and @lodejard.

@BrennanConroy let's remove the number from the enum, and leave "None" at the bottom (because it has to be the largest.) Make sure to put a comment to remind folks keeping "None" at the bottom with any future change.

[NickCraver]

@muratg I don't think removing the numbers is the best approach here. Without numbers, you're going to get automatic numbering and (potentially) break any library that wasn't recompiled to handle the change later and definitely complicate handling of previously serialized data.

Let's say you insert a new level above Critical, if someone has compiled for None previously, they'll be handling a new log level that wasn't intended. This happens for example when serializing the enum as an integer and back (which is far more space efficient for storing things like exceptions). Putting explicit numbers on an enum is precisely how you prevent this situation...I strongly suggest against removing them. Where None lies doesn't bother me much, but it should be constant. If anything though, I'd agree with None being at 0, that's a pretty common convention and the expected position - it also avoids any complications with additions later and (if it were serialized) allows a much smaller integer type to be used (e.g. tinyint in the database rather than a full int - this shouldn't happen in any case I can think of, but that's why you rarely file an enum with a crazy-high value).

[Eilon]

I definitely wouldn't remove the numbers. All enums should always have all values with explicit numbers.

[304NotModified]

I definitely wouldn't remove the numbers. All enums should always have all values with explicit numbers.

Agree with this one.

Somewhat related, in NLog we choose not to use enums at all for the LogLevel, see code. This because of extensibility.

[lodejard]

Okay, chatted with some folks, sounds like the proposal would be adjusted slightly to this straw man:

• Keep the explicit constant numeric values in the declaration
• Start with 0 as Trace so that default(LogLevel) gives you a value that's in the set
• Don't skip to int.MaxValue - instead have None be one higher than Critical.

Trace=0,Debug=1,Information=2,Warning=3,Error=4,Critical=5,None=6

[muratg]

Does this mean if at any point we add a new level, we'll add it at the end with number 7?

[benaadams]

Does this mean if at any point we add a new level, we'll add it at the end with number 7?

Could skip some?

Trace=0,Debug=4,Information=8,Warning=12,Error=16,Critical=20,None=24

[lodejard]

Oh, guys... 😫... Are we certain this is a place that needs future-proofing?

If so I believe the way that turns out is closer to what NLog and log4j have implemented. As an immutable LogLevel class with pre-defined public static constants for the assorted levels, integers that skip by a large number, and convenience methods on the LogLevel class to help correctly implement a mapping from a given LogLevel value to a provider-specific enum member.

See also http://svn.apache.org/viewvc/logging/log4j/trunk/src/main/java/org/apache/log4j/Priority.java?view=markup and http://svn.apache.org/viewvc/logging/log4j/trunk/src/main/java/org/apache/log4j/Level.java?view=markup

Though TBH IMHO if we did all that work I don't predict we'll ever end up doing more than this enum currently represents.

[NickCraver]

@lodejard I don't think we need to future proof that much really, but None should be at 0. That's almost universal in .Net: if there's a None, it's at 0. What's the reason for breaking this expectation? It also avoids the problem of a adding a new logging level at the end looking very weird later as None starts appearing somewhat randomly in the middle of the list.

Trace is no more valid of a default than any other log-something level, but None actually is special and makes complete sense as a default, since you're opting into logging in the first place. That's pretty much why you find None at the start almost everywhere.

[lodejard]

None is used for filtering via >=. As in "log this level or higher". If None was 0 it would actually act as "All".

[cwe1ss]

I agree with @NickCraver - A enum entry of None should have 0 because that's how enums are supposed to work in .NET (just look at CA1008: Enums should have zero value for an example. Giving them a non-zero value is extremely confusing.

Regarding the filtering - I think it depends on the requirement. If you have a MinimumLevel property and None = 0 than one would expect to get no logging if None is specified. This would mean a simple >= comparison is not possible and None needs special treatment.
However if you actually specify a filter yourself through code, I think it's logical to get all logs returned if you manually specify >= LogLevel.None. If you wouldn't want to have logs you would naturally go for = LogLevel.None.

If you want to stick with a non-zero value for "no logging" I would call it Off instead because that would clearly set it apart from the original intend of None.

[lodejard]

Okay, then what is the intended usage of "None" at that point? It doesn't have a meaning as a level of a log message, and it doesn't have a meaning as a filter level.

Should we leave "Trace" as 1, introduce "Off" as higher than "Critical", and change "None" to 0?

This would mean default(LogLevel)==LogLevel.None but that is actually an invalid enum member in both places this enum is used.

[benaadams]

Reverse the enums and invert the tests?

None=0,Critical=1,Error=2,Warning=3,Information=4,Debug=5,Trace=6

Though I suppose you might want to output by default so?

Verbose=0,Trace=1,Debug=2,Information=3,Warning=4,Error=5,Critical=6,None=7

[cwe1ss]

Another option would be to have neither None nor Off and just solve the filtering/MinimumLevel problem in a different way (separate method calls, different enum, ...)? As you've said, both values don't make sense as an actual "log level". This way we could use Trace = 0 which somehow makes sense even if it's used for default(LogLevel) and we could apply filtering as expected: >= Trace would log everything.
This would be a rather big change though.

For that reason I vote for Trace=0, Debug=1, ..., Critical=5 and an entry called Off with a higher value. I don't think we need to be future proof here so the value could be 6 but I'm also ok with any other higher number. This would be an easy solution and a very good compromise in my opinion.

[BrennanConroy]

a6b0cd5