Summary: | build warning for unused function parameters in indexeddb | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Charles Wei <charles.wei> | ||||||||||
Component: | WebCore Misc. | Assignee: | Charles Wei <charles.wei> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | alecflett, dgrogan, jsbell, tony, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Charles Wei
2013-03-22 04:40:54 PDT
Created attachment 194514 [details]
Patch
Created attachment 194822 [details]
Patch
Comment on attachment 194822 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194822&action=review Thanks for the cleanup! > Source/WebCore/Modules/indexeddb/IDBCallbacks.h:71 > + virtual void onBlocked(int64_t /* existingVersion */) { ASSERT_NOT_REACHED(); } It looks like WebKit style is to not have spaces inside the comment, e.g. (int64_t /*existingVersion*/) > Source/WebCore/Modules/indexeddb/IDBCallbacks.h:73 > + virtual void onUpgradeNeeded(int64_t /* oldVersion */, PassRefPtr<IDBDatabaseBackendInterface>, const IDBDatabaseMetadata&) { ASSERT_NOT_REACHED(); } Ditto. > Source/WebCore/Modules/indexeddb/IDBCursor.cpp:300 > +IndexedDB::CursorDirection IDBCursor::stringToDirection(const String& directionString, ScriptExecutionContext*, ExceptionCode& ec) Please just remove the ScriptExecutionContext argument from this method and all callers. It is no longer used now that we've dropped support for legacy constants. > Source/WebCore/Modules/indexeddb/IDBCursorBackendImpl.cpp:223 > +void IDBCursorBackendImpl::prefetchReset(int usedPrefetches, int /* unusedPrefetches */) Remove extra spaces inside /* ... */ > Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp:918 > +void IDBDatabaseBackendImpl::setIndexesReady(int64_t transactionId, int64_t /* objectStoreId */, const Vector<int64_t>& indexIds) Ditto. > Source/WebCore/Modules/indexeddb/IDBLevelDBCoding.cpp:729 > +int compare(const LevelDBSlice& a, const LevelDBSlice& b, bool /* ignoreDuplicates */, bool& ok) Ditto. > Source/WebCore/Modules/indexeddb/IDBLevelDBCoding.cpp:752 > +int compare<ExistsEntryKey>(const LevelDBSlice& a, const LevelDBSlice& b, bool /* ignoreDuplicates */, bool& ok) Ditto. > Source/WebCore/Modules/indexeddb/IDBTransaction.cpp:339 > +IndexedDB::TransactionMode IDBTransaction::stringToMode(const String& modeString, ScriptExecutionContext*, ExceptionCode& ec) Please just remove the ScriptExecutionContext argument from the method and all callers. It is no longer used now that we've dropped support for legacy constants. Created attachment 194995 [details]
Patch
Thanks for the comments, Joshua. About the style for inline comments "/* */", there are no standard style guideline in webkit coding standard about whether to have a white space after leading "/*" and before "*/". There are both styles (with and without the white space) in webkit code, both seem OK. But I personally prefer to have the white space to make the code easier to read. Do you agree, Joshua ? About your other comments to remove unused ScriptExecutionContext, I've done that with the new patch. Thanks. Comment on attachment 194995 [details] Patch Attachment 194995 [details] did not pass win-ews (win): Output: http://webkit-commit-queue.appspot.com/results/17311292 (In reply to comment #5) > About the style for inline comments "/* */", there are no standard style guideline in webkit coding standard about whether to have a white space after leading "/*" and before "*/". There are both styles (with and without the white space) in webkit code, both seem OK. Okay. I guess the files I looked at all used the more compact form. Doesn't matter. > About your other comments to remove unused ScriptExecutionContext, I've done that with the new patch. Cool, looks good to me. tony@ - could you r? this? Comment on attachment 194995 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194995&action=review > Source/WebCore/Modules/indexeddb/IDBCursorBackendImpl.cpp:223 > -void IDBCursorBackendImpl::prefetchReset(int usedPrefetches, int unusedPrefetches) > +void IDBCursorBackendImpl::prefetchReset(int usedPrefetches, int /* unusedPrefetches */) Nit: WebKit style is to normally leave parameters unnamed if they are unused. The name can be found in the header file if you want to know what the param is for. > Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp:918 > -void IDBDatabaseBackendImpl::setIndexesReady(int64_t transactionId, int64_t objectStoreId, const Vector<int64_t>& indexIds) > +void IDBDatabaseBackendImpl::setIndexesReady(int64_t transactionId, int64_t /* objectStoreId */, const Vector<int64_t>& indexIds) Remove objectStoreId > Source/WebCore/Modules/indexeddb/IDBLevelDBCoding.cpp:729 > -int compare(const LevelDBSlice& a, const LevelDBSlice& b, bool ignoreDuplicates, bool& ok) > +int compare(const LevelDBSlice& a, const LevelDBSlice& b, bool /* ignoreDuplicates */, bool& ok) Remove ignoreDuplicates > Source/WebCore/Modules/indexeddb/IDBLevelDBCoding.cpp:752 > -int compare<ExistsEntryKey>(const LevelDBSlice& a, const LevelDBSlice& b, bool ignoreDuplicates, bool& ok) > +int compare<ExistsEntryKey>(const LevelDBSlice& a, const LevelDBSlice& b, bool /* ignoreDuplicates */, bool& ok) Remove ignoreDuplicates > Source/WebCore/Modules/indexeddb/IDBLevelDBCoding.cpp:775 > -int compare<ObjectStoreDataKey>(const LevelDBSlice& a, const LevelDBSlice& b, bool ignoreDuplicates, bool& ok) > +int compare<ObjectStoreDataKey>(const LevelDBSlice& a, const LevelDBSlice& b, bool /* ignoreDuplicates */, bool& ok) Remove ignoreDuplicates > Source/WebCore/inspector/InspectorIndexedDBAgent.cpp:205 > -void ExecutableWithDatabase::start(IDBFactory* idbFactory, SecurityOrigin* securityOrigin, const String& databaseName) > +void ExecutableWithDatabase::start(IDBFactory* idbFactory, SecurityOrigin*, const String& databaseName) This is a good example of WebKit style. Comment on attachment 194995 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194995&action=review >> Source/WebCore/Modules/indexeddb/IDBCursorBackendImpl.cpp:223 >> +void IDBCursorBackendImpl::prefetchReset(int usedPrefetches, int /* unusedPrefetches */) > > Nit: WebKit style is to normally leave parameters unnamed if they are unused. The name can be found in the header file if you want to know what the param is for. Agree. Created attachment 195239 [details]
Patch
Comment on attachment 195239 [details] Patch Clearing flags on attachment: 195239 Committed r147029: <http://trac.webkit.org/changeset/147029> All reviewed patches have been landed. Closing bug. |