[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