[Webkit-unassigned] [Bug 53003] Web Inspector: [JSC] implement setting breakpoints by line:column

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=53003


Rob Buis <rwlbuis at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #143987|review?                     |review-
               Flag|                            |




--- Comment #4 from Rob Buis <rwlbuis at gmail.com>  2012-05-28 12:49:59 PST ---
(From update of attachment 143987)
View in context: https://bugs.webkit.org/attachment.cgi?id=143987&action=review

Looks good, can be improved some more.

> Source/WebCore/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=53003

Better add an empty line here.

> Source/WebCore/ChangeLog:5
> +        The RIM PR https://bugzilla.qnx.com/bugzilla/show_bug.cgi?id=154675 is depend on this bug.

This needs to go below Reviewed by. Also please add an empty line after Reviewed by.

> Source/WebCore/bindings/js/ScriptDebugServer.cpp:75
> +        return true;

I think you can combine below so you only have two returns.

> Source/WebCore/bindings/js/ScriptDebugServer.cpp:525
> +    m_lastHitScriptBreakpoints.clear();

You are doing same lines of code as in returnEvent. So I think there can be a helper function to avoid duplicating code.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.