Issue GoogleCloudPlatform#231 - Implemented suggested changes from re…

…view
jetty-project · Jul 7, 2016 · 5f4683b · 5f4683b
1 parent fdf9586
commit 5f4683b
Show file tree

Hide file tree

Showing 7 changed files with 13 additions and 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
@@ -45,22 +45,22 @@ 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
       String julConfigFile = System.getProperty(JAVA_UTIL_LOGGING_CONFIG_PROPERTY);
       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/...ngine-java-logging/src/test/java/com/google/apphosting/logging/CommonsLoggingExample.java b/...ngine-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
@@ -22,12 +22,10 @@
 import java.util.logging.LogRecord;
 
 public class JsonCaptureHandler extends Handler {
-
-  private JsonFormatter formatter;
+  private final JsonFormatter formatter;
   private List<String> 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
@@ -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
@@ -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
@@ -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
@@ -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