[webkit-reviews] review denied: [Bug 26054] Need a new abstraction layer between the DB classes and the file system : [Attachment 30977] same patch with a minor change

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jun 10 12:41:35 PDT 2009


Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied 's request for
review:
Bug 26054: Need a new abstraction layer between the DB classes and the file
system
https://bugs.webkit.org/show_bug.cgi?id=26054

Attachment 30977: same patch with a minor change
https://bugs.webkit.org/attachment.cgi?id=30977&action=review

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
I looked over existing WebKit code and it looks like the practice of just
defining functions, rather than a static class is preferred. I don't have hard
feelings either way.
Overall, this looks fine. There are a couple of style nits below:


Now that I think about it, since the code in SQLiteFileSystem.* is reused from
other files, we should take the copyright header from those files and add (C)
Google blah line to it.

> Property changes on: WebCore/platform/sql/SQLiteFileSystem.cpp
> ___________________________________________________________________
> Added: svn:executable
>    + *

No need for this svn prop.

> +    /*
> +	* Registers a user-defined SQLite VFS.
> +	*/
> +    static void registerSQLiteVFS();

Use C++ style comments here and elsewhere. No need for extra lines.

>      if (result == SQLResultRow)
> -	   return pathByAppendingComponent(originPath,
statement.getColumnText(0));
> +	 return SQLiteFileSystem::appendDatabaseFileNameToPath(originPath,
statement.getColumnText(0));

A spacing issue?


More information about the webkit-reviews mailing list