You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "erikmav (via GitHub)" <gi...@apache.org> on 2024/03/15 19:35:16 UTC
[PR] Nullability and modernization updates, part 3 (logging-log4net)
erikmav opened a new pull request, #126:
URL: https://github.com/apache/logging-log4net/pull/126
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Nullability and modernization updates, part 3 (logging-log4net)
Posted by "erikmav (via GitHub)" <gi...@apache.org>.
erikmav commented on PR #126:
URL: https://github.com/apache/logging-log4net/pull/126#issuecomment-2000343172
@FreeAndNil Important to closely review `ReadOnlyPropertiesDictionary` and `PropertiesDictionary`. I selected the most backward compatible option, which is to coerce the `this[]` getter to avoid throwing KeyNotFoundException like Dictionary<> would (including a new test for the behavior). Also note that to avoid a double-indirection it would be ideal to change both to structs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Nullability and modernization updates, part 3 (logging-log4net)
Posted by "erikmav (via GitHub)" <gi...@apache.org>.
erikmav commented on PR #126:
URL: https://github.com/apache/logging-log4net/pull/126#issuecomment-2000331267
For #124
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Nullability and modernization updates, part 3 (logging-log4net)
Posted by "FreeAndNil (via GitHub)" <gi...@apache.org>.
FreeAndNil commented on code in PR #126:
URL: https://github.com/apache/logging-log4net/pull/126#discussion_r1529138434
##########
src/log4net/Core/LoggingEvent.cs:
##########
@@ -826,11 +825,10 @@ private static string TryReadWindowsIdentityUserName()
{
try
{
- if (System.Threading.Thread.CurrentPrincipal is not null &&
- System.Threading.Thread.CurrentPrincipal.Identity is not null &&
- System.Threading.Thread.CurrentPrincipal.Identity.Name is not null)
+ if (Thread.CurrentPrincipal is not null &&
Review Comment:
I think we can shorten this even more:
```suggestion
if (Thread.CurrentPrincipal?.Identity?.Name is string name)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Nullability and modernization updates, part 3 (logging-log4net)
Posted by "FreeAndNil (via GitHub)" <gi...@apache.org>.
FreeAndNil commented on code in PR #126:
URL: https://github.com/apache/logging-log4net/pull/126#discussion_r1528410182
##########
src/log4net/Core/LoggingEvent.cs:
##########
@@ -840,44 +800,41 @@ private string TryGetCurrentUserName()
}
}
- private string _cachedWindowsIdentityUserName;
+ private string? _cachedWindowsIdentityUserName;
private static string TryReadWindowsIdentityUserName()
{
using var identity = WindowsIdentity.GetCurrent();
- return identity?.Name ?? "";
+ return identity?.Name ?? string.Empty;
}
private static bool _platformDoesNotSupportWindowsIdentity;
/// <summary>
/// Gets the identity of the current thread principal.
/// </summary>
- /// <value>
- /// The string name of the identity of the current thread principal.
- /// </value>
/// <remarks>
/// <para>
/// Calls <c>System.Threading.Thread.CurrentPrincipal.Identity.Name</c> to get
/// the name of the current thread principal.
/// </para>
/// </remarks>
- public string Identity
+ public string? Identity
{
get
{
- if (m_data.Identity == null && this.m_cacheUpdatable)
+ if (m_data.Identity is null && m_cacheUpdatable)
{
try
{
- if (System.Threading.Thread.CurrentPrincipal != null &&
- System.Threading.Thread.CurrentPrincipal.Identity != null &&
- System.Threading.Thread.CurrentPrincipal.Identity.Name != null)
+ if (System.Threading.Thread.CurrentPrincipal is not null &&
+ System.Threading.Thread.CurrentPrincipal.Identity is not null &&
+ System.Threading.Thread.CurrentPrincipal.Identity.Name is not null)
Review Comment:
```suggestion
System.Threading.Thread.CurrentPrincipal?.Identity?.Name is string name)
```
##########
src/log4net/Core/LoggingEvent.cs:
##########
@@ -840,44 +800,41 @@ private string TryGetCurrentUserName()
}
}
- private string _cachedWindowsIdentityUserName;
+ private string? _cachedWindowsIdentityUserName;
private static string TryReadWindowsIdentityUserName()
{
using var identity = WindowsIdentity.GetCurrent();
- return identity?.Name ?? "";
+ return identity?.Name ?? string.Empty;
}
private static bool _platformDoesNotSupportWindowsIdentity;
/// <summary>
/// Gets the identity of the current thread principal.
/// </summary>
- /// <value>
- /// The string name of the identity of the current thread principal.
- /// </value>
/// <remarks>
/// <para>
/// Calls <c>System.Threading.Thread.CurrentPrincipal.Identity.Name</c> to get
/// the name of the current thread principal.
/// </para>
/// </remarks>
- public string Identity
+ public string? Identity
{
get
{
- if (m_data.Identity == null && this.m_cacheUpdatable)
+ if (m_data.Identity is null && m_cacheUpdatable)
{
try
{
- if (System.Threading.Thread.CurrentPrincipal != null &&
- System.Threading.Thread.CurrentPrincipal.Identity != null &&
- System.Threading.Thread.CurrentPrincipal.Identity.Name != null)
+ if (System.Threading.Thread.CurrentPrincipal is not null &&
+ System.Threading.Thread.CurrentPrincipal.Identity is not null &&
+ System.Threading.Thread.CurrentPrincipal.Identity.Name is not null)
{
m_data.Identity = System.Threading.Thread.CurrentPrincipal.Identity.Name;
Review Comment:
```suggestion
m_data.Identity = name;
```
##########
src/log4net/Repository/Hierarchy/Hierarchy.cs:
##########
@@ -378,33 +333,36 @@ public override ILogger GetLogger(string name)
/// </para>
/// <para>
/// The <c>Shutdown</c> method is careful to close nested
- /// appenders before closing regular appenders. This is allows
+ /// appenders before closing regular appenders. This allows
/// configurations where a regular appender is attached to a logger
/// and again to a nested appender.
/// </para>
/// </remarks>
public override void Shutdown()
{
- LogLog.Debug(declaringType, "Shutdown called on Hierarchy [" + this.Name + "]");
+ LogLog.Debug(declaringType, $"Shutdown called on Hierarchy [{Name}]");
// begin by closing nested appenders
Root.CloseNestedAppenders();
- lock (m_ht)
- {
- ILogger[] currentLoggers = this.GetCurrentLoggers();
+ ILogger[] currentLoggers = GetCurrentLoggers();
- foreach (Logger logger in currentLoggers)
+ foreach (ILogger logger in currentLoggers)
+ {
+ if (logger is Logger l)
{
- logger.CloseNestedAppenders();
+ l.CloseNestedAppenders();
}
+ }
- // then, remove all appenders
- Root.RemoveAllAppenders();
+ // then, remove all appenders
+ Root.RemoveAllAppenders();
- foreach (Logger logger in currentLoggers)
+ foreach (ILogger logger in currentLoggers)
Review Comment:
same as above
##########
src/log4net/Repository/Hierarchy/XmlHierarchyConfigurator.cs:
##########
@@ -90,70 +84,78 @@ public void Configure(XmlElement element)
if (rootElementName != CONFIGURATION_TAG)
{
- LogLog.Error(declaringType, "Xml element is - not a <" + CONFIGURATION_TAG + "> element.");
+ LogLog.Error(declaringType, $"Xml element is - not a <{CONFIGURATION_TAG}> element.");
return;
}
if (!LogLog.EmitInternalMessages)
{
// Look for a emitDebug attribute to enable internal debug
string emitDebugAttribute = element.GetAttribute(EMIT_INTERNAL_DEBUG_ATTR);
- LogLog.Debug(declaringType, EMIT_INTERNAL_DEBUG_ATTR + " attribute [" + emitDebugAttribute + "].");
+ LogLog.Debug(declaringType, $"{EMIT_INTERNAL_DEBUG_ATTR} attribute [{emitDebugAttribute}].");
if (emitDebugAttribute.Length > 0 && emitDebugAttribute != "null")
{
LogLog.EmitInternalMessages = OptionConverter.ToBoolean(emitDebugAttribute, true);
}
else
{
- LogLog.Debug(declaringType, "Ignoring " + EMIT_INTERNAL_DEBUG_ATTR + " attribute.");
+ LogLog.Debug(declaringType, $"Ignoring {EMIT_INTERNAL_DEBUG_ATTR} attribute.");
}
}
if (!LogLog.InternalDebugging)
{
// Look for a debug attribute to enable internal debug
string debugAttribute = element.GetAttribute(INTERNAL_DEBUG_ATTR);
- LogLog.Debug(declaringType, INTERNAL_DEBUG_ATTR + " attribute [" + debugAttribute + "].");
+ LogLog.Debug(declaringType, $"{INTERNAL_DEBUG_ATTR} attribute [{debugAttribute}].");
if (debugAttribute.Length > 0 && debugAttribute != "null")
{
LogLog.InternalDebugging = OptionConverter.ToBoolean(debugAttribute, true);
}
else
{
- LogLog.Debug(declaringType, "Ignoring " + INTERNAL_DEBUG_ATTR + " attribute.");
+ LogLog.Debug(declaringType, $"Ignoring {INTERNAL_DEBUG_ATTR} attribute.");
}
string confDebug = element.GetAttribute(CONFIG_DEBUG_ATTR);
if (confDebug.Length > 0 && confDebug != "null")
{
- LogLog.Warn(declaringType, "The \"" + CONFIG_DEBUG_ATTR + "\" attribute is deprecated.");
- LogLog.Warn(declaringType, "Use the \"" + INTERNAL_DEBUG_ATTR + "\" attribute instead.");
+ LogLog.Warn(declaringType, $"The \"{CONFIG_DEBUG_ATTR}\" attribute is deprecated.");
+ LogLog.Warn(declaringType, $"Use the \"{INTERNAL_DEBUG_ATTR}\" attribute instead.");
LogLog.InternalDebugging = OptionConverter.ToBoolean(confDebug, true);
}
}
// Default mode is merge
- ConfigUpdateMode configUpdateMode = ConfigUpdateMode.Merge;
+ ConfigUpdateMode? configUpdateMode = ConfigUpdateMode.Merge;
// Look for the config update attribute
- string configUpdateModeAttribute = element.GetAttribute(CONFIG_UPDATE_MODE_ATTR);
- if (configUpdateModeAttribute != null && configUpdateModeAttribute.Length > 0)
+ string? configUpdateModeAttribute = element.GetAttribute(CONFIG_UPDATE_MODE_ATTR);
+ if (!string.IsNullOrEmpty(configUpdateModeAttribute))
{
// Parse the attribute
try
{
- configUpdateMode = (ConfigUpdateMode)OptionConverter.ConvertStringTo(typeof(ConfigUpdateMode), configUpdateModeAttribute);
+ var mode = (ConfigUpdateMode?)OptionConverter.ConvertStringTo(typeof(ConfigUpdateMode), configUpdateModeAttribute);
+ if (mode is not null)
Review Comment:
```suggestion
if (OptionConverter.ConvertStringTo(typeof(ConfigUpdateMode), configUpdateModeAttribute) is ConfigUpdateMode mode)
```
##########
src/log4net/Util/PatternConverter.cs:
##########
@@ -214,9 +185,12 @@ public virtual void Format(TextWriter writer, object state)
}
}
- private static readonly string[] SPACES = { " ", " ", " ", " ", // 1,2,4,8 spaces
- " ", // 16 spaces
- " " }; // 32 spaces
+ private static readonly string[] SPACES =
+ {
+ " ", " ", " ", " ", // 1,2,4,8 spaces
+ " ", // 16 spaces
+ " "
+ }; // 32 spaces
Review Comment:
please move the comment one line up
##########
src/log4net/Util/ThreadContextStack.cs:
##########
@@ -239,7 +224,7 @@ public override string ToString()
/// </remarks>
object IFixingRequired.GetFixedObject()
Review Comment:
```suggestion
object IFixingRequired.GetFixedObject() => GetFullMessage() ?? string.Empty;
```
##########
src/log4net/Util/ThreadContextProperties.cs:
##########
@@ -116,13 +97,9 @@ public void Remove(string key)
/// Gets the keys stored in the properties.
/// </para>
/// <returns>a set of the defined keys</returns>
- public string[] GetKeys()
+ public string[]? GetKeys()
Review Comment:
```suggestion
public string[]? GetKeys() => _dictionary?.GetKeys();
```
##########
src/log4net/Repository/Hierarchy/Hierarchy.cs:
##########
@@ -378,33 +333,36 @@ public override ILogger GetLogger(string name)
/// </para>
/// <para>
/// The <c>Shutdown</c> method is careful to close nested
- /// appenders before closing regular appenders. This is allows
+ /// appenders before closing regular appenders. This allows
/// configurations where a regular appender is attached to a logger
/// and again to a nested appender.
/// </para>
/// </remarks>
public override void Shutdown()
{
- LogLog.Debug(declaringType, "Shutdown called on Hierarchy [" + this.Name + "]");
+ LogLog.Debug(declaringType, $"Shutdown called on Hierarchy [{Name}]");
// begin by closing nested appenders
Root.CloseNestedAppenders();
- lock (m_ht)
- {
- ILogger[] currentLoggers = this.GetCurrentLoggers();
+ ILogger[] currentLoggers = GetCurrentLoggers();
- foreach (Logger logger in currentLoggers)
+ foreach (ILogger logger in currentLoggers)
+ {
+ if (logger is Logger l)
Review Comment:
```suggestion
```
##########
src/log4net/Repository/Hierarchy/Hierarchy.cs:
##########
@@ -494,38 +454,34 @@ public override void Log(LoggingEvent logEvent)
/// The list returned is unordered but does not contain duplicates.
/// </para>
/// </remarks>
- public override Appender.IAppender[] GetAppenders()
+ public override IAppender[] GetAppenders()
{
- System.Collections.ArrayList appenderList = new System.Collections.ArrayList();
+ var appenderList = new List<IAppender>();
CollectAppenders(appenderList, Root);
- foreach (Logger logger in GetCurrentLoggers())
+ foreach (ILogger logger in GetCurrentLoggers())
Review Comment:
same as above
##########
src/log4net/Repository/Hierarchy/Hierarchy.cs:
##########
@@ -378,33 +333,36 @@ public override ILogger GetLogger(string name)
/// </para>
/// <para>
/// The <c>Shutdown</c> method is careful to close nested
- /// appenders before closing regular appenders. This is allows
+ /// appenders before closing regular appenders. This allows
/// configurations where a regular appender is attached to a logger
/// and again to a nested appender.
/// </para>
/// </remarks>
public override void Shutdown()
{
- LogLog.Debug(declaringType, "Shutdown called on Hierarchy [" + this.Name + "]");
+ LogLog.Debug(declaringType, $"Shutdown called on Hierarchy [{Name}]");
// begin by closing nested appenders
Root.CloseNestedAppenders();
- lock (m_ht)
- {
- ILogger[] currentLoggers = this.GetCurrentLoggers();
+ ILogger[] currentLoggers = GetCurrentLoggers();
- foreach (Logger logger in currentLoggers)
+ foreach (ILogger logger in currentLoggers)
Review Comment:
```suggestion
foreach (Logger logger in currentLoggers.OfType<Logger>())
```
##########
src/log4net/Repository/Hierarchy/Hierarchy.cs:
##########
@@ -937,24 +849,27 @@ private static void UpdateChildren(ProvisionNode pn, Logger log)
/// <param name="levelEntry">the level values</param>
/// <remarks>
/// <para>
- /// Define or redefine a Level using the values in the <see cref="LevelEntry"/> argument
- /// </para>
- /// <para>
/// Supports setting levels via the configuration file.
/// </para>
/// </remarks>
internal void AddLevel(LevelEntry levelEntry)
{
- if (levelEntry == null) throw new ArgumentNullException("levelEntry");
- if (levelEntry.Name == null) throw new ArgumentNullException("levelEntry.Name");
+ if (levelEntry is null)
+ {
+ throw new ArgumentNullException(nameof(levelEntry));
+ }
+ if (levelEntry.Name is null)
+ {
+ throw new ArgumentNullException("levelEntry.Name");
+ }
// Lookup replacement value
if (levelEntry.Value == -1)
{
- Level previousLevel = LevelMap[levelEntry.Name];
- if (previousLevel == null)
+ Level? previousLevel = LevelMap[levelEntry.Name];
+ if (previousLevel is null)
Review Comment:
```suggestion
if (LevelMap[levelEntry.Name] is Level previousLevel)
```
##########
src/log4net/Util/OptionConverter.cs:
##########
@@ -280,8 +261,8 @@ public static object ConvertTypeTo(object sourceInstance, Type targetType)
}
// Look for a TO converter
- IConvertTo tcSource = ConverterRegistry.GetConvertTo(sourceType, targetType);
- if (tcSource != null)
+ IConvertTo? tcSource = ConverterRegistry.GetConvertTo(sourceType, targetType);
+ if (tcSource is not null)
Review Comment:
same as above
##########
src/log4net/Core/LoggingEvent.cs:
##########
@@ -1295,23 +1229,19 @@ protected virtual void FixVolatileData(FixFlags flags)
}
// avoid warning CS0219
- if (forceCreation != null)
+ if (forceCreation is not null)
Review Comment:
Can you please remove forceCreation in this method and replace it with _ ?
##########
src/log4net/Core/LoggingEvent.cs:
##########
@@ -1320,7 +1250,7 @@ private void CreateCompositeProperties()
{
compositeProperties.Add(logicalThreadProperties);
}
- var threadProperties = ThreadContext.Properties.GetProperties(false);
+ PropertiesDictionary threadProperties = ThreadContext.Properties.GetProperties(false);
if (threadProperties != null)
Review Comment:
```suggestion
if (ThreadContext.Properties.GetProperties(false) is PropertiesDictionary threadProperties)
```
##########
src/log4net/Util/OptionConverter.cs:
##########
@@ -246,8 +227,8 @@ public static bool CanConvertTypeTo(Type sourceType, Type targetType)
}
// Look for a From converter
- IConvertFrom tcTarget = ConverterRegistry.GetConvertFrom(targetType);
- if (tcTarget != null)
+ IConvertFrom? tcTarget = ConverterRegistry.GetConvertFrom(targetType);
+ if (tcTarget is not null)
Review Comment:
same as above
##########
src/log4net/Repository/Hierarchy/Hierarchy.cs:
##########
@@ -216,11 +188,7 @@ public Hierarchy(PropertiesDictionary properties, ILoggerFactory loggerFactory)
/// about not having an appender warning.
/// </para>
/// </remarks>
- public bool EmittedNoAppenderWarning
- {
- get { return m_emittedNoAppenderWarning; }
- set { m_emittedNoAppenderWarning = value; }
- }
+ public bool EmittedNoAppenderWarning { get; set; } = false;
Review Comment:
```suggestion
public bool EmittedNoAppenderWarning { get; set; }
```
##########
src/log4net/Util/OptionConverter.cs:
##########
@@ -196,8 +177,8 @@ public static object ConvertStringTo(Type target, string txt)
// to an arbitrary type T there will be a static method defined on type T called Parse
// that will take an argument of type string. i.e. T.Parse(string)->T we call this
// method to convert the string to the type required by the property.
- System.Reflection.MethodInfo meth = target.GetMethod("Parse", new Type[] { typeof(string) });
- if (meth != null)
+ MethodInfo? meth = target.GetMethod("Parse", new[] { typeof(string) });
+ if (meth is not null)
Review Comment:
```suggestion
if (target.GetMethod("Parse", new[] { typeof(string) }) is MethodInfo meth)
```
##########
src/log4net/Repository/Hierarchy/Hierarchy.cs:
##########
@@ -436,12 +394,11 @@ public override void ResetConfiguration()
Root.Level = LevelMap.LookupWithDefault(Level.Debug);
Threshold = LevelMap.LookupWithDefault(Level.All);
- // the synchronization is needed to prevent hashtable surprises
- lock (m_ht)
- {
- Shutdown(); // nested locks are OK
+ Shutdown(); // nested locks are OK
- foreach (Logger l in this.GetCurrentLoggers())
+ foreach (ILogger logger in GetCurrentLoggers())
Review Comment:
same as above
##########
src/log4net/Util/OptionConverter.cs:
##########
@@ -236,8 +217,8 @@ public static bool CanConvertTypeTo(Type sourceType, Type targetType)
}
// Look for a To converter
- IConvertTo tcSource = ConverterRegistry.GetConvertTo(sourceType, targetType);
- if (tcSource != null)
+ IConvertTo? tcSource = ConverterRegistry.GetConvertTo(sourceType, targetType);
+ if (tcSource is not null)
Review Comment:
same as above
##########
src/log4net/Util/ThreadContextStack.cs:
##########
@@ -223,7 +208,7 @@ internal Stack InternalStack
/// Gets the current context information for this stack.
/// </para>
/// </remarks>
- public override string ToString()
+ public override string? ToString()
Review Comment:
```suggestion
public override string? ToString() => GetFullMessage();
```
##########
src/log4net/Util/ReadOnlyPropertiesDictionary.cs:
##########
@@ -178,12 +192,7 @@ public bool Contains(string key)
/// The hashtable used to store the properties
/// </para>
/// </remarks>
- protected Hashtable InnerHashtable
- {
- get { return m_hashtable; }
- }
-
- #region Implementation of ISerializable
+ protected Dictionary<string, object?> InnerHashtable { get; } = new(StringComparer.Ordinal);
Review Comment:
@fluffynuts I've added this breaking change to the release notes:
https://github.com/apache/logging-log4net/pull/115/commits/db44c9cefd47464407475f28288c97fe0bb1dbe6
##########
src/log4net/Util/OptionConverter.cs:
##########
@@ -290,50 +271,18 @@ public static object ConvertTypeTo(object sourceInstance, Type targetType)
}
// Look for a FROM converter
- IConvertFrom tcTarget = ConverterRegistry.GetConvertFrom(targetType);
- if (tcTarget != null)
+ IConvertFrom? tcTarget = ConverterRegistry.GetConvertFrom(targetType);
+ if (tcTarget is not null)
Review Comment:
same as above
##########
src/log4net/Util/TypeConverters/ConverterRegistry.cs:
##########
@@ -208,24 +180,19 @@ public static IConvertFrom GetConvertFrom(Type destinationType)
/// The type converter instance to use for type conversions or <c>null</c>
/// if no type converter is found.
/// </returns>
- private static object GetConverterFromAttribute(Type destinationType)
+ private static object? GetConverterFromAttribute(Type destinationType)
{
// Look for an attribute on the destination type
- var attributes = destinationType
- .GetCustomAttributes(typeof(TypeConverterAttribute), true);
- if (attributes is null)
- {
- // I assume the original null check is perhaps for CF or older .NET versions -- please leave in place
- return null;
- }
-
+ object[] attributes = destinationType.GetCustomAttributes(typeof(TypeConverterAttribute), true);
foreach (var attribute in attributes)
{
- var tcAttr = attribute as TypeConverterAttribute;
- if (tcAttr != null)
+ if (attribute is TypeConverterAttribute tcAttr)
{
- var converterType = SystemInfo.GetTypeFromString(destinationType, tcAttr.ConverterTypeName, false, true);
- return CreateConverterInstance(converterType);
+ Type? converterType = SystemInfo.GetTypeFromString(destinationType, tcAttr.ConverterTypeName, false, true);
+ if (converterType is not null)
Review Comment:
```suggestion
if (SystemInfo.GetTypeFromString(destinationType, tcAttr.ConverterTypeName, false, true) is Type converterType)
```
##########
src/log4net/Util/ThreadContextProperties.cs:
##########
@@ -135,20 +112,13 @@ public string[] GetKeys()
/// </remarks>
public void Clear()
Review Comment:
```suggestion
public void Clear() => _dictionary?.Clear();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Nullability and modernization updates, part 3 (logging-log4net)
Posted by "FreeAndNil (via GitHub)" <gi...@apache.org>.
FreeAndNil commented on PR #126:
URL: https://github.com/apache/logging-log4net/pull/126#issuecomment-2004768377
> Also note that to avoid a double-indirection it would be ideal to change both to structs.
@erikmav can you explain how you would implement this? Maybe as separate issue?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Nullability and modernization updates, part 3 (logging-log4net)
Posted by "erikmav (via GitHub)" <gi...@apache.org>.
erikmav commented on PR #126:
URL: https://github.com/apache/logging-log4net/pull/126#issuecomment-2005755352
Could be done under another issue. The change is to change from class to struct (or readonly struct, depending) for ReadOnlyPropertiesDictionary, then derive PropertiesDictionary from that as a second struct. I wouldn't do it in this PR either way, it's mainly an optimization.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Nullability and modernization updates, part 3 (logging-log4net)
Posted by "FreeAndNil (via GitHub)" <gi...@apache.org>.
FreeAndNil merged PR #126:
URL: https://github.com/apache/logging-log4net/pull/126
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Nullability and modernization updates, part 3 (logging-log4net)
Posted by "FreeAndNil (via GitHub)" <gi...@apache.org>.
FreeAndNil commented on PR #126:
URL: https://github.com/apache/logging-log4net/pull/126#issuecomment-2000364779
Thanks. I will review on Monday.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org