C++ Readability review for holte

components/rappor/log_uploader.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// Change for readability
+
 #ifndef COMPONENTS_RAPPOR_LOG_UPLOADER_H_
 #define COMPONENTS_RAPPOR_LOG_UPLOADER_H_
 
 #include <queue>
 #include <string>
 
+#include "base/compiler_specific.h"
+#include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "net/url_request/url_fetcher_delegate.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "url/gurl.h"
 
 namespace net {
 class URLFetcher;
 }
 
 namespace rappor {
 
 // Handles uploading logs to an external server.

[ktl, 2014/03/18 08:35:01]

This class could use a longer comment. When do uploads happen, for example?
Where is this class used? What happens to queued comments when the instance is
destructed? (That last thing could be added to the destructor documentation.)

[Steven Holte, 2014/03/19 23:46:55]

Done

 class LogUploader : public net::URLFetcherDelegate {
  public:
   // Constructor takes the server_url that logs should be uploaded to, the

[ktl, 2014/03/18 08:35:01]

In other places you use |name| convention for variable names.

[Steven Holte, 2014/03/19 23:46:55]

Done.

   // mime_type of the uploaded data, and request_context to create uploads
   // with.
   LogUploader(const GURL& server_url,
               const std::string& mime_type,
               net::URLRequestContextGetter* request_context);
 
   virtual ~LogUploader();
 
   // Adds an entry to the queue of logs to be uploaded to the server.  The
   // uploader makes no assumptions about the format of |log| and simply sends
   // it verbatim to the server.
   void QueueLog(const std::string& log);
 
  protected:
   // Check if an upload has been scheduled.

[ktl, 2014/03/18 08:35:01]

"Checks", for consistency.

[Steven Holte, 2014/03/19 23:46:55]

Done.

   virtual bool IsUploadScheduled() const;
 
   // Schedules a future call to StartScheduledUpload if one isn't already
   // pending.  Can be overridden for testing.
   virtual void ScheduleNextUpload(base::TimeDelta interval);
 
   // Starts transmission of the next log. Exposed for tests.
   void StartScheduledUpload();
 
   // Increases the upload interval each time it's called, to handle the case
@@ skipping 32 matching lines @@
   // The interval to wait after an upload's URLFetcher completion before
   // starting the next upload attempt.
   base::TimeDelta upload_interval_;
 
   DISALLOW_COPY_AND_ASSIGN(LogUploader);
 };
 
 }  // namespace rappor
 
 #endif  // COMPONENTS_RAPPOR_LOG_UPLOADER_H_