[webkit-reviews] review granted: [Bug 177027] Web Inspector: Enable WebKit logging configuration and display : [Attachment 324015] Rebased patch.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Oct 20 15:02:41 PDT 2017


Joseph Pecoraro <joepeck at webkit.org> has granted Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 177027: Web Inspector: Enable WebKit logging configuration and display
https://bugs.webkit.org/show_bug.cgi?id=177027

Attachment 324015: Rebased patch.

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




--- Comment #34 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 324015
  --> https://bugs.webkit.org/attachment.cgi?id=324015
Rebased patch.

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

r=me

> Source/JavaScriptCore/inspector/scripts/codegen/generator.py:49
> +    'webrtc': 'WebRTC',  # Canvas.ContextType.webrtc

Heh this isn't Canvas. Comment should be Console.ChannelSource.webrtc

> Source/WebInspectorUI/UserInterface/Controllers/LogManager.js:42
> +	       this._loggingChannelSources = [
WI.ConsoleMessage.MessageSource.Media, WI.ConsoleMessage.MessageSource.WebRTC
];

Style: Avoid leading and trailing padding spaces in the array literal.

> Source/WebInspectorUI/UserInterface/Models/LoggingChannel.js:34
> +	   console.assert(level === WI.LoggingChannel.Level.Off || level ===
WI.LoggingChannel.Level.Always || level === WI.LoggingChannel.Level.Error ||
level === WI.LoggingChannel.Level.Warning || level ===
WI.LoggingChannel.Level.Notice || level === WI.LoggingChannel.Level.Info ||
level === WI.LoggingChannel.Level.Debug);

Style: A compact way to do this is:

    console.assert(Object.values(WI.LoggingChannel.Level).includes(level));

> Source/WebKitLegacy/mac/WebCoreSupport/WebChromeClient.mm:399
> +

Style: Extra empty line.


More information about the webkit-reviews mailing list