[webkit-reviews] review denied: [Bug 73798] Upstream 6 files into WebCore/platform/blackberry : [Attachment 118354] Patch

[bugzilla-daemon at webkit.org]

Rob Buis <rwlbuis at gmail.com> has denied Mary Wu <mary.wu at torchmobile.com.cn>'s
request for review:
Bug 73798: Upstream 6 files into WebCore/platform/blackberry
https://bugs.webkit.org/show_bug.cgi?id=73798

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

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


I am pretty sure these are the last remaining problems, but for now r-.

> Source/WebCore/platform/blackberry/FileSystemBlackBerry.cpp:50
> +    DIR* dir;

You could move this to when they are actually used. For instance dir is only
used much later. This is inconsistent with how de variable is used, which is
only declared just before it is used.

> Source/WebCore/platform/blackberry/FileSystemBlackBerry.cpp:54
> +    // if directory part don't have '/' in the end, add it

Should be made into a proper sentence starting with Capital and ending with
punctuation mark. Also probably should be placed before the if.

> Source/WebCore/platform/blackberry/SharedTimerBlackBerry.cpp:45
> +static SharedTimerBlackBerry* s_timer = 0;

I think it is nicer to move this into the instance() method. Also then you can
remove the s_ prefix.