[webkit-reviews] review denied: [Bug 119548] Refactoring of throw exception. : [Attachment 209914] patch 7
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Aug 28 14:18:13 PDT 2013
Geoffrey Garen <ggaren at apple.com> has denied Chris Curtis
<chris_curtis at apple.com>'s request for review:
Bug 119548: Refactoring of throw exception.
https://bugs.webkit.org/show_bug.cgi?id=119548
Attachment 209914: patch 7
https://bugs.webkit.org/attachment.cgi?id=209914&action=review
------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=209914&action=review
I would normally mark this r+, and then let you resolve these comments before
landing, but since you're using the commit queue, you'll need to upload a new
patch to land.
> Source/JavaScriptCore/runtime/VM.cpp:595
> + // No range information, so give a few characters of context
Let's upgrade this to a complete sentence by adding a "." at the end.
> Source/JavaScriptCore/runtime/VM.cpp:601
> + // then strip whitespace.
Let's upgrade this to a complete sentence by capitalizing "Then".
> Source/JavaScriptCore/runtime/VM.cpp:637
> + // FIXME: should only really be adding these properties to VM
generated exceptions,
Let's upgrade this to a complete sentence by adding a "We".
> LayoutTests/fast/xmlhttprequest/xmlhttprequest-recursive-sync-event.html:94
> + do{
Need a space after "do" here.
> LayoutTests/fast/xmlhttprequest/xmlhttprequest-recursive-sync-event.html:98
> + hasCompensatedAlready++ ;
> + } while (hasCompensatedAlready == 1 && lastReadyStateObserved ==
1);
Why did hasCompensatedAlready change to a count when we're only trying to count
from 0 to 1? That seems like a boolean to me. If it is a real count, we should
rename hasCompensatedAlready to something like "compensationCount", to indicate
that it's a count.
More information about the webkit-reviews
mailing list