You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "FreeAndNil (via GitHub)" <gi...@apache.org> on 2024/10/23 11:49:25 UTC
[PR] Enable Microsoft.CodeAnalysis.NetAnalyzers (logging-log4net)
FreeAndNil opened a new pull request, #201:
URL: https://github.com/apache/logging-log4net/pull/201
activated Microsoft.CodeAnalysis.NetAnalyzers and fixed Warnings
--
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] Enable Microsoft.CodeAnalysis.NetAnalyzers (logging-log4net)
Posted by "FreeAndNil (via GitHub)" <gi...@apache.org>.
FreeAndNil commented on code in PR #201:
URL: https://github.com/apache/logging-log4net/pull/201#discussion_r1814398417
##########
src/log4net.Tests/Appender/AdoNet/Log4NetCommand.cs:
##########
@@ -24,7 +24,7 @@
namespace log4net.Tests.Appender.AdoNet;
-public class Log4NetCommand : IDbCommand
+internal sealed class Log4NetCommand : IDbCommand
Review Comment:
Yes, but this is in our test assembly and we don't ship it ;-)
This class should better be named MockDbCommand - I will change it in my next PR.
--
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] Enable Microsoft.CodeAnalysis.NetAnalyzers (logging-log4net)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #201:
URL: https://github.com/apache/logging-log4net/pull/201#discussion_r1814394802
##########
src/log4net.Tests/Appender/AdoNet/Log4NetCommand.cs:
##########
@@ -24,7 +24,7 @@
namespace log4net.Tests.Appender.AdoNet;
-public class Log4NetCommand : IDbCommand
+internal sealed class Log4NetCommand : IDbCommand
Review Comment:
Just a curiosity: isn't this a breaking change?
At least it would be a breaking change in Java.
--
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] Enable Microsoft.CodeAnalysis.NetAnalyzers (logging-log4net)
Posted by "FreeAndNil (via GitHub)" <gi...@apache.org>.
FreeAndNil merged PR #201:
URL: https://github.com/apache/logging-log4net/pull/201
--
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] Enable Microsoft.CodeAnalysis.NetAnalyzers (logging-log4net)
Posted by "FreeAndNil (via GitHub)" <gi...@apache.org>.
FreeAndNil commented on code in PR #201:
URL: https://github.com/apache/logging-log4net/pull/201#discussion_r1867328985
##########
src/log4net/ObjectRenderer/RendererMap.cs:
##########
@@ -217,7 +211,7 @@ public IObjectRenderer Get(Type type)
/// <summary>
/// Gets the default renderer instance
/// </summary>
- public IObjectRenderer DefaultRenderer => _sDefaultRenderer;
+ public static IObjectRenderer DefaultRenderer { get; } = new DefaultRenderer();
Review Comment:
reason for #211
--
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] Enable Microsoft.CodeAnalysis.NetAnalyzers (logging-log4net)
Posted by "gdziadkiewicz (via GitHub)" <gi...@apache.org>.
gdziadkiewicz commented on code in PR #201:
URL: https://github.com/apache/logging-log4net/pull/201#discussion_r1858355376
##########
src/log4net/ObjectRenderer/RendererMap.cs:
##########
@@ -217,7 +211,7 @@ public IObjectRenderer Get(Type type)
/// <summary>
/// Gets the default renderer instance
/// </summary>
- public IObjectRenderer DefaultRenderer => _sDefaultRenderer;
+ public static IObjectRenderer DefaultRenderer { get; } = new DefaultRenderer();
Review Comment:
For record: this change in public API broke log4net 3.0.3 for log4net.Ext.Json 3.0.1.
--
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