[webkit-reviews] review denied: [Bug 42426] Implement remaining Inspector support for chrome's appcache : [Attachment 62253] patch

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has denied Kavita Kanetkar
<kkanetkar at chromium.org>'s request for review:
Bug 42426: Implement remaining Inspector support for chrome's appcache
https://bugs.webkit.org/show_bug.cgi?id=42426

Attachment 62253: patch
https://bugs.webkit.org/attachment.cgi?id=62253&action=review

------- Additional Comments from Joseph Pecoraro <joepeck at webkit.org>
> +	       CacheInfo(const KURL& manifest, double creationTime, double
updateTime, long long size)
> +		   : m_manifest(manifest)
> +		   , m_creationTime(creationTime)
> +		   , m_updateTime(updateTime)
> +		   , m_size(size) {   }

> +	       ResourceInfo(const KURL& resource, bool isMaster, bool
isManifest, bool isFallback, bool isForeign, bool isExplicit, long long size)
> +		   : m_resource(resource)
> +		   , m_isMaster(isMaster)
> +		   , m_isManifest(isManifest)
> +		   , m_isFallback(isFallback)
> +		   , m_isForeign(isForeign)
> +		   , m_isExplicit(isExplicit)
> +		   , m_size(size) {  }

You changed the format here. I don't know the correct style for an empty
constructor like this. I put the braces on individual lines before, I
have also seen braces with a single space in between "{ }". This change
is neither of these. I'd suggest looking around at some other structs
and matching the style of those for empty constructors.

Since there are comments that need to be addressed by Michael and you will
need to put up another patch anyways I'll set r-.