You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2021/06/03 13:14:59 UTC
[GitHub] [logging-log4j2] vy commented on a change in pull request #502: Improvements:
vy commented on a change in pull request #502:
URL: https://github.com/apache/logging-log4j2/pull/502#discussion_r644751295
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/FileExtension.java
##########
@@ -95,7 +95,7 @@ public static FileExtension lookupForFile(final String fileName) {
private final String extension;
- private FileExtension(final String extension) {
+ FileExtension(final String extension) {
Review comment:
Why did you increase this ctor's visibility?
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/time/internal/format/FixedDateFormat.java
##########
@@ -324,11 +324,11 @@ public int getSecondFractionDigits() {
*/
HHCMM(':', true, 6);
- private FixedTimeZoneFormat() {
+ FixedTimeZoneFormat() {
Review comment:
Why not `private`?
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncQueueFullPolicyFactory.java
##########
@@ -81,9 +81,9 @@ private static boolean isRouterSelected(
String propertyValue,
Class<? extends AsyncQueueFullPolicy> policy,
String shortPropertyValue) {
- return propertyValue != null && (shortPropertyValue.equalsIgnoreCase(propertyValue)
+ return shortPropertyValue.equalsIgnoreCase(propertyValue)
Review comment:
I think the former approach short-circuits earlier when `propertyValue` is `null` rather than traversing the entire `||` operands.
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/layout/HtmlLayout.java
##########
@@ -72,13 +72,13 @@
private final DatePatternConverter datePatternConverter;
/**Possible font sizes */
- public static enum FontSize {
+ public enum FontSize {
SMALLER("smaller"), XXSMALL("xx-small"), XSMALL("x-small"), SMALL("small"), MEDIUM("medium"), LARGE("large"),
XLARGE("x-large"), XXLARGE("xx-large"), LARGER("larger");
private final String size;
- private FontSize(final String size) {
+ FontSize(final String size) {
Review comment:
Why not `private`?
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/tools/picocli/CommandLine.java
##########
@@ -4402,8 +4401,10 @@ public static IStyle bg(String str) {
public String off() { return CSI + (fgbg + 1) + "m"; }
}
private static class StyledSection {
- int startIndex, length;
- String startStyles, endStyles;
+ int startIndex;
+ final int length;
+ String startStyles;
+ final String endStyles;
Review comment:
These all can very well be `private final`, I guess.
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/time/internal/format/FixedDateFormat.java
##########
@@ -324,11 +324,11 @@ public int getSecondFractionDigits() {
*/
HHCMM(':', true, 6);
- private FixedTimeZoneFormat() {
+ FixedTimeZoneFormat() {
this(NONE, true, 4);
}
- private FixedTimeZoneFormat(final char timeSeparatorChar, final boolean minutes, final int length) {
+ FixedTimeZoneFormat(final char timeSeparatorChar, final boolean minutes, final int length) {
Review comment:
Why not `private`?
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/tools/picocli/CommandLine.java
##########
@@ -4668,8 +4669,8 @@ private void print(Tracer tracer, String msg, Object... params) {
static TraceLevel lookup(String key) { return key == null ? WARN : empty(key) || "true".equalsIgnoreCase(key) ? INFO : valueOf(key); }
}
private static class Tracer {
- TraceLevel level = TraceLevel.lookup(System.getProperty("picocli.trace"));
- PrintStream stream = System.err;
+ final TraceLevel level = TraceLevel.lookup(System.getProperty("picocli.trace"));
+ final PrintStream stream = System.err;
Review comment:
Can't these be `private` as well?
##########
File path: log4j-core/src/test/java/org/apache/logging/log4j/core/async/perftest/PerfTestDriver.java
##########
@@ -192,14 +192,14 @@ public String toString() {
}
}
- static enum Runner {
+ enum Runner {
Log4j12(RunLog4j1.class), //
Log4j2(RunLog4j2.class), //
Logback(RunLogback.class);
private final Class<? extends IPerfTestRunner> implementationClass;
- private Runner(final Class<? extends IPerfTestRunner> cls) {
+ Runner(final Class<? extends IPerfTestRunner> cls) {
Review comment:
Why not `private`?
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/net/Rfc1349TrafficClass.java
##########
@@ -39,7 +39,7 @@
private final int trafficClass;
- private Rfc1349TrafficClass(final int trafficClass) {
+ Rfc1349TrafficClass(final int trafficClass) {
Review comment:
Why not `private`?
##########
File path: log4j-core/src/test/java/org/apache/logging/log4j/core/time/internal/format/FastDateParserTest.java
##########
@@ -628,12 +627,12 @@ private static Calendar initializeCalendar(final TimeZone tz) {
return cal;
}
- private static enum Expected1806 {
+ private enum Expected1806 {
India(INDIA, "+05", "+0530", "+05:30", true),
Greenwich(GMT, "Z", "Z", "Z", false),
NewYork(NEW_YORK, "-05", "-0500", "-05:00", false);
- private Expected1806(final TimeZone zone, final String one, final String two, final String three, final boolean hasHalfHourOffset) {
+ Expected1806(final TimeZone zone, final String one, final String two, final String three, final boolean hasHalfHourOffset) {
Review comment:
Why not `private`?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org