[webkit-reviews] review denied: [Bug 43151] Add WebKit API for HTML5 FileSystem API : [Attachment 62887] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Jul 28 18:13:42 PDT 2010
Dumitru Daniliuc <dumi at chromium.org> has denied Kinuko Yasuda
<kinuko at chromium.org>'s request for review:
Bug 43151: Add WebKit API for HTML5 FileSystem API
https://bugs.webkit.org/show_bug.cgi?id=43151
Attachment 62887: Patch
https://bugs.webkit.org/attachment.cgi?id=62887&action=review
------- Additional Comments from Dumitru Daniliuc <dumi at chromium.org>
WebKit/chromium/ChangeLog:16
+ * public/WebGetFileFlags.h: Added.
if you haven't done so already, please ask darin if he's ok with the names of
these files.
WebKit/chromium/public/WebAsyncFileSystem.h:44
+ class WebAsyncFileSystem {
it looks to me like this class is not referenced from anywhere, and all its
methods are stubbed out. i think it might be better to move this .h file to a
patch that implements or uses this class.
WebKit/chromium/public/WebFSCallbacks.h:42
+ class WebFSCallbacks {
same comment.
WebKit/chromium/public/WebFileError.h:34
+ #include "WebCommon.h"
is this include needed?
WebKit/chromium/public/WebFileError.h:47
+ enum Code {
how about renaming Code to FileErrorCode?
WebKit/chromium/public/WebFileError.h:50
+ // NO_MODIFICATION_ALLOWED_ERR FileError in HTML5 File API.
i don't think you need a comment for every error code. i think it should be
enough to mention once that these are the FileError codes from the HTML5 File
API.
WebKit/chromium/public/WebGetFileFlags.h:39
+ // If |create| is true, WebAsyncFileSystem.getFile or getDirectory
creates a file or directory if it was not previously there.
nit: s/WebAsyncFileSystem.getFile/WebAsyncFileSystem::getFile()/,
s/getDirectory/getDirectory()/.
More information about the webkit-reviews
mailing list