garydgregory commented on a change in pull request #789:
URL: https://github.com/apache/logging-log4j2/pull/789#discussion_r825295699
##########
File path:
log4j-1.2-api/src/main/java/org/apache/log4j/xml/XmlConfiguration.java
##########
@@ -675,24 +675,13 @@ private void parseLevel(Element element, LoggerConfig
logger, boolean isRoot) {
}
} else {
String className = subst(element.getAttribute(CLASS_ATTR));
+ final Level level;
if (EMPTY_STR.equals(className)) {
- logger.setLevel(OptionConverter.convertLevel(priStr,
org.apache.logging.log4j.Level.DEBUG));
+ level = OptionConverter.toLevel(priStr, Level.DEBUG);
Review comment:
THere are several places where we specify a default level (Level.DEBUG)
in this method and likely elsewhere. It would be nicer to have a default level
defined in a constant IMO; this would make it obvious that this is a "default"
as opposed to "in this case, we have level Foo". WDYT?
##########
File path:
log4j-1.2-api/src/main/java/org/apache/log4j/xml/XmlConfiguration.java
##########
@@ -675,24 +675,13 @@ private void parseLevel(Element element, LoggerConfig
logger, boolean isRoot) {
}
} else {
String className = subst(element.getAttribute(CLASS_ATTR));
+ final Level level;
if (EMPTY_STR.equals(className)) {
- logger.setLevel(OptionConverter.convertLevel(priStr,
org.apache.logging.log4j.Level.DEBUG));
+ level = OptionConverter.toLevel(priStr, Level.DEBUG);
Review comment:
There are several places where we specify a default level (Level.DEBUG)
in this method and likely elsewhere. It would be nicer to have a default level
defined in a constant IMO; this would make it obvious that this is a "default"
as opposed to "in this case, we have level Foo". WDYT?
--
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-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
