From 5f4683b26b257df02f1cfc8abdd9fff5cde0b221 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 7 Jul 2016 15:06:14 -0700 Subject: [PATCH] Issue #231 - Implemented suggested changes from review --- .../google/apphosting/logging/CoreLogging.java | 16 ++++++++-------- .../logging/CommonsLoggingExample.java | 2 +- .../apphosting/logging/JsonCaptureHandler.java | 4 +--- .../apphosting/logging/JsonFormatterTest.java | 1 - .../google/apphosting/logging/JulExample.java | 2 +- .../google/apphosting/logging/Log4jExample.java | 2 +- .../google/apphosting/logging/Slf4jExample.java | 2 +- 7 files changed, 13 insertions(+), 16 deletions(-) diff --git a/appengine-java-logging/src/main/java/com/google/apphosting/logging/CoreLogging.java b/appengine-java-logging/src/main/java/com/google/apphosting/logging/CoreLogging.java index afba806..71d179f 100644 --- a/appengine-java-logging/src/main/java/com/google/apphosting/logging/CoreLogging.java +++ b/appengine-java-logging/src/main/java/com/google/apphosting/logging/CoreLogging.java @@ -45,7 +45,7 @@ public class CoreLogging { public static void init(File appConfigFile) throws IOException { // Use App (User) Configuration specified as a file parameter if (appConfigFile != null && appConfigFile.exists()) { - debug("Loading User Config (from file): %s", appConfigFile); + debug("Loading App Config (from file): %s", appConfigFile); appConfig(appConfigFile); } else { // Use App (User) Configuration specified as a System property @@ -53,14 +53,14 @@ public static void init(File appConfigFile) throws IOException { if (julConfigFile != null) { File configFile = new File(julConfigFile); if (configFile.exists()) { - debug("Loading User Config (from property): %s", appConfigFile); + debug("Loading App Config (from property): %s", appConfigFile); appConfig(configFile); } else { - warning("Logging Config System Property (%s) points to invalid file: %s", + warning("Config System Property (%s) points to invalid file: %s", JAVA_UTIL_LOGGING_CONFIG_PROPERTY, appConfigFile.getAbsolutePath()); } } else { - debug("No User Config"); + debug("No App Config"); } } @@ -69,7 +69,7 @@ public static void init(File appConfigFile) throws IOException { } /** - * Convenience method for {@link #init(File)} + * Convenience method for {@link #init(File)}. * * @param appConfigFilename the filename of the config file (or null) * @throws IOException if unable to configure the logging @@ -88,14 +88,14 @@ private static void appConfig(File configFile) { logManager.reset(); // close & remove existing handlers, reset existing logger levels logManager.readConfiguration(is); } catch (SecurityException | IOException e) { - System.err.println("Warning: caught exception when reading logging properties: " + configFile + warning("Warning: caught exception when reading logging properties: %s", configFile .getAbsolutePath()); e.printStackTrace(System.err); } } /** - * Manually Configure the System Level requirements for java.util.logging + * Manually Configure the System Level requirements for java.util.logging. */ private static void systemConfig() throws IOException { // Since System Loggers can arrive from deep within the various compat layers, it @@ -109,7 +109,7 @@ private static void systemConfig() throws IOException { systemLogger.configure(); count++; } - debug("Ran %d SystemLogger(s)", count); + debug("Initialized %d SystemLogger(s)", count); } private static void debug(String format, Object... args) { diff --git a/appengine-java-logging/src/test/java/com/google/apphosting/logging/CommonsLoggingExample.java b/appengine-java-logging/src/test/java/com/google/apphosting/logging/CommonsLoggingExample.java index a46227b..82645d2 100644 --- a/appengine-java-logging/src/test/java/com/google/apphosting/logging/CommonsLoggingExample.java +++ b/appengine-java-logging/src/test/java/com/google/apphosting/logging/CommonsLoggingExample.java @@ -21,7 +21,7 @@ import java.util.concurrent.ThreadLocalRandom; -public class CommonsLoggingExample implements Runnable { +class CommonsLoggingExample implements Runnable { private static final Log LOG = LogFactory.getLog(CommonsLoggingExample.class); @Override diff --git a/appengine-java-logging/src/test/java/com/google/apphosting/logging/JsonCaptureHandler.java b/appengine-java-logging/src/test/java/com/google/apphosting/logging/JsonCaptureHandler.java index b54d2e2..36dba2c 100644 --- a/appengine-java-logging/src/test/java/com/google/apphosting/logging/JsonCaptureHandler.java +++ b/appengine-java-logging/src/test/java/com/google/apphosting/logging/JsonCaptureHandler.java @@ -22,12 +22,10 @@ import java.util.logging.LogRecord; public class JsonCaptureHandler extends Handler { - - private JsonFormatter formatter; + private final JsonFormatter formatter; private List events; public JsonCaptureHandler() { - super(); formatter = new JsonFormatter(); events = new ArrayList<>(); } diff --git a/appengine-java-logging/src/test/java/com/google/apphosting/logging/JsonFormatterTest.java b/appengine-java-logging/src/test/java/com/google/apphosting/logging/JsonFormatterTest.java index 7586ca8..bf49331 100644 --- a/appengine-java-logging/src/test/java/com/google/apphosting/logging/JsonFormatterTest.java +++ b/appengine-java-logging/src/test/java/com/google/apphosting/logging/JsonFormatterTest.java @@ -126,7 +126,6 @@ public void testCommonsLogging() { for (int i = 0; i < events.size(); i++) { String logLine = events.get(i); - System.out.printf("logLine[%d] = %s", i, logLine); JsonData data = new Gson().fromJson(logLine, JsonData.class); assertThat("severity", data.severity, is(expected[i][0])); assertThat("timestamp.seconds", data.timestamp.seconds, diff --git a/appengine-java-logging/src/test/java/com/google/apphosting/logging/JulExample.java b/appengine-java-logging/src/test/java/com/google/apphosting/logging/JulExample.java index 9493e17..3421834 100644 --- a/appengine-java-logging/src/test/java/com/google/apphosting/logging/JulExample.java +++ b/appengine-java-logging/src/test/java/com/google/apphosting/logging/JulExample.java @@ -20,7 +20,7 @@ import java.util.logging.Level; import java.util.logging.Logger; -public class JulExample implements Runnable { +class JulExample implements Runnable { public static final Logger LOG = Logger.getLogger(JulExample.class.getName()); @Override diff --git a/appengine-java-logging/src/test/java/com/google/apphosting/logging/Log4jExample.java b/appengine-java-logging/src/test/java/com/google/apphosting/logging/Log4jExample.java index 74c6761..157bb03 100644 --- a/appengine-java-logging/src/test/java/com/google/apphosting/logging/Log4jExample.java +++ b/appengine-java-logging/src/test/java/com/google/apphosting/logging/Log4jExample.java @@ -20,7 +20,7 @@ import java.util.concurrent.ThreadLocalRandom; -public class Log4jExample implements Runnable { +class Log4jExample implements Runnable { private static final Logger LOG = Logger.getLogger(Log4jExample.class); @Override diff --git a/appengine-java-logging/src/test/java/com/google/apphosting/logging/Slf4jExample.java b/appengine-java-logging/src/test/java/com/google/apphosting/logging/Slf4jExample.java index d2354cf..0cf4e64 100644 --- a/appengine-java-logging/src/test/java/com/google/apphosting/logging/Slf4jExample.java +++ b/appengine-java-logging/src/test/java/com/google/apphosting/logging/Slf4jExample.java @@ -21,7 +21,7 @@ import java.util.concurrent.ThreadLocalRandom; -public class Slf4jExample implements Runnable { +class Slf4jExample implements Runnable { private static final Logger LOG = LoggerFactory.getLogger(Slf4jExample.class); @Override