C++ readability review from original change https://chromiumcodereview.appspot.com/14263024/

chrome/test/chromedriver/chrome/log.h

Index: chrome/test/chromedriver/chrome/log.h
diff --git a/chrome/test/chromedriver/chrome/log.h b/chrome/test/chromedriver/chrome/log.h
index 31c18466b39bc6697735320e0d9fdc3a1da92273..eca8e652199ba8482acfbcc64ea16fce9f992ce9 100644
--- a/chrome/test/chromedriver/chrome/log.h
+++ b/chrome/test/chromedriver/chrome/log.h
@@ -11,9 +11,10 @@ namespace base {
 class Time;
 }
 
-// Accepts log entries that have a level, timestamp, and a string message.
+// Abstract class for logging entries with a level, timestamp, string message.
 class Log {
  public:
+  // Log entry severity level.
   enum Level {
     kDebug,
     kLog,
@@ -23,13 +24,13 @@ class Log {
 
   virtual ~Log() {}
 
-  // Log a message with an explicit timestamp.
+  // Logs a message with an explicit timestamp.
   virtual void AddEntry(const base::Time& time,
                         Level level,
                         const std::string& message) = 0;
 
-  // Implicit timestamp, default to current time.
-  void AddEntry(Level level, const std::string& message);
+  // Logs a message with an implicit timestamp, default to current time.
+  virtual void AddEntry(Level level, const std::string& message);

[jdennett, 2013/05/23 03:44:24]

It's also strange to have overload sets of virtual functions!

Generally a virtual function should not be overloaded, and should be the single
customization point.

I can live with this as is, but it's a touch awkward.

[klm, 2013/05/23 15:21:39]

The idea was that callers can add a log entry with an explicit timestamp, or
implicitly stamped as "now". This has caused me more trouble than it's worth --
C++ is a lot less overload-friendly than Java, so I now removed the overload and
everyone must pass a timestamp, although it's allowed to be null.

 };
 
 #endif  // CHROME_TEST_CHROMEDRIVER_CHROME_LOG_H_