[webkit-reviews] review denied: [Bug 88678] IndexedDB: Add new type (and chromium webkit API) for metadata snapshot : [Attachment 147103] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Jun 14 22:56:38 PDT 2012
Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Joshua Bell
<jsbell at chromium.org>'s request for review:
Bug 88678: IndexedDB: Add new type (and chromium webkit API) for metadata
snapshot
https://bugs.webkit.org/show_bug.cgi?id=88678
Attachment 147103: Patch
https://bugs.webkit.org/attachment.cgi?id=147103&action=review
------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=147103&action=review
> Source/WebKit/chromium/public/WebIDBKeyPath.h:48
> WEBKIT_EXPORT WebIDBKeyPath(const WebIDBKeyPath&);
nit: the normal pattern is to implement copy constructor and assignment
operator
inline in terms of an assign function. they don't need to be exported then.
see WebNode.h for example.
> Source/WebKit/chromium/public/WebIDBMetadata.h:31
> +#include "platform/WebPrivateOwnPtr.h"
nit: looks like you don't need this header.
> Source/WebKit/chromium/public/WebIDBMetadata.h:35
> +namespace WebCore {
looks like you don't need these forward declarations
> Source/WebKit/chromium/public/WebIDBMetadata.h:50
> + WebIDBMetadata()
nit: no need to write this constructor
> Source/WebKit/chromium/public/WebIDBMetadata.h:58
> + WebVector<Index> indexes;
nit: indexes -> indices?
> Source/WebKit/chromium/public/WebIDBMetadata.h:60
> + : name(WebString())
nit: no need for explicit constructor here
> Source/WebKit/chromium/public/WebIDBMetadata.h:71
> + : name(WebString())
ditto
More information about the webkit-reviews
mailing list