[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