From 3db7a775c97d9f001d56a1b8d328b6ee624a1566 Mon Sep 17 00:00:00 2001
From: Erik Mavrinac <erikmav@microsoft.com>
Date: Thu, 28 Mar 2024 11:20:20 -0700
Subject: [PATCH] Nullable: Eliminate nullable in one place, and clarify the
 meaning of null levels in doc comments (#136)

---
 .../Appender/EventLogAppenderTest.cs          |  2 +-
 src/log4net/Appender/AppenderSkeleton.cs      | 13 +++---
 src/log4net/Core/LoggingEvent.cs              | 43 +++++++++----------
 3 files changed, 26 insertions(+), 32 deletions(-)

diff --git a/src/log4net.Tests/Appender/EventLogAppenderTest.cs b/src/log4net.Tests/Appender/EventLogAppenderTest.cs
index 2339ad68..2382118a 100644
--- a/src/log4net.Tests/Appender/EventLogAppenderTest.cs
+++ b/src/log4net.Tests/Appender/EventLogAppenderTest.cs
@@ -85,7 +85,7 @@ public void TestGetEntryTypeForExistingApplicationName()
     [Ignore("seems to require administrator privileges or a specific environment when run")]
     public void ActivateOptionsDisablesAppenderIfSourceDoesntExist()
     {
-      EventLogAppender eventAppender = new EventLogAppender();
+      var eventAppender = new EventLogAppender();
       eventAppender.ActivateOptions();
       Assert.AreEqual(Level.Off, eventAppender.Threshold);
     }
diff --git a/src/log4net/Appender/AppenderSkeleton.cs b/src/log4net/Appender/AppenderSkeleton.cs
index 02592718..64cfa4f2 100644
--- a/src/log4net/Appender/AppenderSkeleton.cs
+++ b/src/log4net/Appender/AppenderSkeleton.cs
@@ -79,6 +79,7 @@ protected AppenderSkeleton()
 
     /// <summary>
     /// Gets or sets the threshold <see cref="Level"/> of this appender.
+    /// Defaults to <see cref="Level.All"/>.
     /// </summary>
     /// <value>
     /// The threshold <see cref="Level"/> of the appender. 
@@ -94,7 +95,7 @@ protected AppenderSkeleton()
     /// string, such as "DEBUG", "INFO" and so on.
     /// </para>
     /// </remarks>
-    public Level? Threshold { get; set; }
+    public Level Threshold { get; set; } = Level.All;
 
     /// <summary>
     /// Gets or sets the <see cref="IErrorHandler"/> for this appender.
@@ -488,18 +489,14 @@ public virtual void ClearFilters()
     /// Checks if the message level is below this appender's threshold.
     /// </summary>
     /// <param name="level"><see cref="Level"/> to test against.</param>
-    /// <remarks>
-    /// <para>
-    /// If there is no threshold set, then the return value is always <c>true</c>.
-    /// </para>
-    /// </remarks>
     /// <returns>
     /// <c>true</c> if the <paramref name="level"/> meets the <see cref="Threshold"/> 
-    /// requirements of this appender.
+    /// requirements of this appender. A null level always maps to <c>true</c>,
+    /// the equivalent of <see cref="Level.All"/>.
     /// </returns>
     protected virtual bool IsAsSevereAsThreshold(Level? level)
     {
-      return Threshold is null || level is null || level >= Threshold;
+      return level is null || level >= Threshold;
     }
 
     /// <summary>
diff --git a/src/log4net/Core/LoggingEvent.cs b/src/log4net/Core/LoggingEvent.cs
index e80eb3e4..9f67b967 100644
--- a/src/log4net/Core/LoggingEvent.cs
+++ b/src/log4net/Core/LoggingEvent.cs
@@ -39,11 +39,6 @@ public struct LoggingEventData
     /// <summary>
     /// The logger name.
     /// </summary>
-    /// <remarks>
-    /// <para>
-    /// The logger name.
-    /// </para>
-    /// </remarks>
     public string? LoggerName;
 
     /// <summary>
@@ -51,9 +46,13 @@ public struct LoggingEventData
     /// </summary>
     /// <remarks>
     /// <para>
-    /// Level of logging event. Level cannot be Serializable
-    /// because it is a flyweight.  Due to its special serialization it
-    /// cannot be declared final either.
+    /// A null level produces varying results depending on the appenders in use.
+    /// In many cases it is equivalent of <see cref="Level.All"/>, other times
+    /// it is mapped to Debug or Info defaults.
+    /// </para>
+    /// <para>
+    /// Level cannot be Serializable because it is a flyweight.
+    /// Due to its special serialization it cannot be declared final either.
     /// </para>
     /// </remarks>
     public Level? Level;
@@ -61,25 +60,15 @@ public struct LoggingEventData
     /// <summary>
     /// The application supplied message.
     /// </summary>
-    /// <remarks>
-    /// <para>
-    /// The application supplied message of logging event.
-    /// </para>
-    /// </remarks>
     public string? Message;
 
     /// <summary>
-    /// The name of thread
+    /// Gets or sets the name of the thread in which this logging event was generated.
     /// </summary>
-    /// <remarks>
-    /// <para>
-    /// The name of thread in which this logging event was generated
-    /// </para>
-    /// </remarks>
     public string? ThreadName;
 
     /// <summary>
-    /// Gets or sets the local time the event was logged
+    /// Gets or sets the local time the event was logged.
     /// </summary>
     /// <remarks>
     /// <para>
@@ -91,7 +80,7 @@ public struct LoggingEventData
     public DateTime TimeStamp;
 
     /// <summary>
-    /// Gets or sets the UTC time the event was logged
+    /// Gets or sets the UTC time the event was logged.
     /// </summary>
     /// <remarks>
     /// <para>
@@ -227,13 +216,18 @@ public class LoggingEvent : ISerializable
     /// the stack boundary into the logging system for this call.</param>
     /// <param name="repository">The repository this event is logged in.</param>
     /// <param name="loggerName">The name of the logger of this event.</param>
-    /// <param name="level">The level of this event.</param>
+    /// <param name="level">
+    /// The level of this event.
+    /// A null level produces varying results depending on the appenders in use.
+    /// In many cases it is equivalent of <see cref="Level.All"/>, other times
+    /// it is mapped to Debug or Info defaults.
+    /// </param>
     /// <param name="message">The message of this event.</param>
     /// <param name="exception">The exception for this event.</param>
     /// <remarks>
     /// <para>
     /// Except <see cref="TimeStamp"/>, <see cref="Level"/> and <see cref="LoggerName"/>, 
-    /// all fields of <c>LoggingEvent</c> are filled when actually needed. Call
+    /// all fields of <c>LoggingEvent</c> are lazily filled when actually needed. Call
     /// <see cref="M:FixVolatileData()"/> to cache all data locally
     /// to prevent inconsistencies.
     /// </para>
@@ -438,6 +432,9 @@ protected LoggingEvent(SerializationInfo info, StreamingContext context)
 
     /// <summary>
     /// Gets the <see cref="Level" /> of the logging event.
+    /// A null level produces varying results depending on the appenders in use.
+    /// In many cases it is equivalent of <see cref="Level.All"/>, other times
+    /// it is mapped to Debug or Info defaults.
     /// </summary>
     public Level? Level => m_data.Level;