[webkit-reviews] review denied: [Bug 30520] Enable creation of custom SidebarTreeElements for different ProfileTypes : [Attachment 41493] patch (fixed)

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Alexander Pavlov (apavlov)
<apavlov at chromium.org>'s request for review:
Bug 30520: Enable creation of custom SidebarTreeElements for different
ProfileTypes
https://bugs.webkit.org/show_bug.cgi?id=30520

Attachment 41493: patch (fixed)
https://bugs.webkit.org/attachment.cgi?id=41493&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
> +    // Must be implemented by subclasses.
> +    createView: function(profile)

Nit, can be done in subsequent patch: usually abstract methods that are
implemented in concrete classes are named as doCreateSomething*.
Otherwise it is easy to get confused on what should be overriden and what not.

> +    {
> +	   throw new Error("Needs implemented.");
> +    },

"Not implemented" ?

>  
> +	   profile._type = profileType;

This looks like a hack: what if concrete profile object already has this
property?
Either declare it a hack and name it "__profilesPanelProfileType" or make
profile define "type" getter and put it into docs.