Bug 147537

Summary:

Web Inspector: Convert CSSStyleDeclarationSection to ES6 Class

Product:

WebKit

Reporter:

Devin Rousso <hi>

Component:

Web Inspector

Assignee:

Devin Rousso <hi>

Status:

RESOLVED INVALID

Severity:

Normal

CC:

graouts, joepeck, jonowells, mattbaker, nvasilyev, timothy, webkit-bug-importer

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch	none

Description Devin Rousso 2015-08-02 00:13:55 PDT

The rest of the CSSStyleSidebar classes are ES6 classes except for CSSStyleDeclarationSection.  We should change that.

Comment 1 Devin Rousso 2015-08-02 00:45:35 PDT

Created attachment 258027 [details]
Patch

Comment 2 Joseph Pecoraro 2015-08-03 15:19:38 PDT

Comment on attachment 258027 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.js:28
> +    constructor(delegate, style) {

Style: Treat "constructor" like a method, and put the opening brace on its own line.

> Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.js:33
> +        console.assert(style);

Nit: I know this is not new, but it is better to assert not just that "style" exists, but that it adheres to some expected type. Perhaps:

    console.assert(style instanceof WebInspector.CSSStyleDeclaration);

Comment 3 Devin Rousso 2015-08-03 23:09:49 PDT

Created attachment 258157 [details]
Patch

Comment 4 Joseph Pecoraro 2015-08-03 23:37:35 PDT

Note, if you have already gotten a review, you can put up the new patch with the "reviewed by" line filled in, and just cq+ it for the commit-queue to land.

Comment 5 Devin Rousso 2015-08-03 23:38:05 PDT

(In reply to comment #4)
> Note, if you have already gotten a review, you can put up the new patch with
> the "reviewed by" line filled in, and just cq+ it for the commit-queue to
> land.

I did not know that.  Thanks for the tip!

Comment 6 Joseph Pecoraro 2015-08-03 23:38:20 PDT

Comment on attachment 258157 [details]
Patch

r=me

Comment 7 Nikita Vasilyev 2015-08-03 23:40:48 PDT

(In reply to comment #4)
> Note, if you have already gotten a review, you can put up the new patch with
> the "reviewed by" line filled in, and just cq+ it for the commit-queue to
> land.

The review dropdown must be left empty. If you set it to "r+", the commit queue will fail.

Comment 8 Timothy Hatcher 2015-08-07 10:09:00 PDT

Comment on attachment 258157 [details]
Patch

It looks like Brian got this. It is now a class in TOT.