[webkit-reviews] review denied: [Bug 76761] [BlackBerry] Credential backing store implementation : [Attachment 123421] Patch

[bugzilla-daemon at webkit.org]

Rob Buis <rwlbuis at gmail.com> has denied Jonathan Dong
<jonathan.dong at torchmobile.com.cn>'s request for review:
Bug 76761: [BlackBerry] Credential backing store implementation
https://bugs.webkit.org/show_bug.cgi?id=76761

Attachment 123421: Patch
https://bugs.webkit.org/attachment.cgi?id=123421&action=review

------- Additional Comments from Rob Buis <rwlbuis at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=123421&action=review


I think it can be improved a bit more, r- for now.

> Source/WebCore/platform/network/blackberry/CredentialBackingStore.cpp:66
> +	   // create table logins

In  general: Make comments look like sentences by starting with a capital
letter and ending with a period.

> Source/WebCore/platform/network/blackberry/CredentialBackingStore.cpp:104
> +    // prepare the statments.

statements.

> Source/WebCore/platform/network/blackberry/CredentialBackingStore.cpp:105
> +

Why empty line?

> Source/WebCore/platform/network/blackberry/CredentialBackingStore.cpp:144
> +    m_updateStatement =0;

= 0

> Source/WebCore/platform/network/blackberry/CredentialBackingStore.cpp:254
> +

Why empty line?

> Source/WebCore/platform/network/blackberry/CredentialBackingStore.cpp:255
> +    return credential;

Why do we need a temp var here?

> Source/WebCore/platform/network/blackberry/CredentialBackingStore.cpp:288
> +	   return false;

This pattern is repeated a lot. Can you come up with macro(s)? If done
correctly it will be much shorter and you don't need braces (go into macro).