[webkit-reviews] review denied: [Bug 27514] add support for watched expressions : [Attachment 39648] proposed patch - 2009/09/16

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at hatcher.name> has denied Patrick Mueller
<pmuellr at yahoo.com>'s request for review:
Bug 27514: add support for watched expressions
https://bugs.webkit.org/show_bug.cgi?id=27514

Attachment 39648: proposed patch - 2009/09/16
https://bugs.webkit.org/attachment.cgi?id=39648&action=review

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>
> +WebInspector.ObjectPropertiesSection.PropertyComparer = function(propertyA,
propertyB) 

WebInspector.ObjectPropertiesSection.CompareProperties would be a better name.
PropertyComparer sounds more like a class name.


> +};

No need for the semicolon.


> +    
>  WebInspector.ObjectPropertiesSection.prototype = {

The compare function you added should come after the prototype, so the
prototype and the constructor are next to each other.


> +    updateProperties: function(properties, rootTreeElementConstructor,
propertyComparer)

I think propertyComparer should be rootPropertyComparer since it is only used
for root items.


> +WebInspector.WatchExpressionsSection.NEW_WATCH_EXPRESSION = "\xA0";

This should not be all caps, it should be camelcase.


> +WebInspector.WatchExpressionsSection.PropertyComparer = function(propertyA,
propertyB) 

Ditto (re: the PropertyComparer name)


> +};

No need for the semicolon.


You also need to add WatchExpressionsSidebarPane.js to the
inspector/front-end/WebKit.qrc, WebCore.gypi and WebCore.vcproj/WebCore.vcproj
for the other platforms.