[webkit-reviews] review denied: [Bug 88678] IndexedDB: Add new type (and chromium webkit API) for metadata snapshot : [Attachment 146621] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sun Jun 10 23:05:32 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 146621: Patch
https://bugs.webkit.org/attachment.cgi?id=146621&action=review
------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=146621&action=review
> Source/WebKit/chromium/public/WebIDBMetadata.h:68
> +class WebIDBObjectStoreMetadata {
nit: one file per top-level class, sorry :(
> Source/WebKit/chromium/public/WebIDBMetadata.h:93
> + WEBKIT_EXPORT WebIDBIndexMetadata(const WebIDBIndexMetadata&);
nit: the usual pattern is to export an assign method instead, and just inline
the constructor as a call to assign.
is there a reason to hide the default constructor and destructor? it seems a
bit odd to hide those using WEBKIT_IMPLEMENTATION. what are you trying to
achieve by doing that? normally WEBKIT_IMPLEMENTATION is only used to hide
methods that use WebCore types.
More information about the webkit-reviews
mailing list