[webkit-reviews] review denied: [Bug 172250] Web Inspector: Use ES2015 Modules for Settings tab : [Attachment 310455] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed May 17 17:07:36 PDT 2017


Joseph Pecoraro <joepeck at webkit.org> has denied Devin Rousso
<drousso at apple.com>'s request for review:
Bug 172250: Web Inspector: Use ES2015 Modules for Settings tab
https://bugs.webkit.org/show_bug.cgi?id=172250

Attachment 310455: Patch

https://bugs.webkit.org/attachment.cgi?id=310455&action=review




--- Comment #2 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 310455
  --> https://bugs.webkit.org/attachment.cgi?id=310455
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=310455&action=review

r-, this won't work with copy-user-interface-resources.pl in macOS Production
builds.

I think we also need to have actual performance numbers to determine if this
path forward is worthwhile or not.

    • A way to measure "time it takes to open web inspector"
    • See a measurable improvement after converting multiple tabs to
dependencies like this.

> Source/WebInspectorUI/ChangeLog:23
> +	   (export.default.SettingEditor):
> +	   (export.default.SettingEditor.createForSetting):
> +	   (export.default.SettingEditor.prototype.set value):
> +	   (export.default.SettingEditor.prototype._createEditorElement):
> +	   (WebInspector.SettingEditor): Deleted.
> +	   (WebInspector.SettingEditor.createForSetting): Deleted.
> +	   (WebInspector.SettingEditor.prototype.get element): Deleted.
> +	   (WebInspector.SettingEditor.prototype.get type): Deleted.
> +	   (WebInspector.SettingEditor.prototype.get label): Deleted.
> +	   (WebInspector.SettingEditor.prototype.get value): Deleted.
> +	   (WebInspector.SettingEditor.prototype.set value): Deleted.
> +	   (WebInspector.SettingEditor.prototype._createEditorElement):
Deleted.

Eliminate all the unnecessary spam in the ChangeLog now that we are just
changing a namespace.

> Source/WebInspectorUI/UserInterface/Views/SettingsGroup.js:69
> -};
> +}

Shouldn't these semicolon be here? Based on the eslintrc.


More information about the webkit-reviews mailing list