[webkit-reviews] review denied: [Bug 189533] Web Inspector: Cookies view should use model objects instead of raw payload data : [Attachment 349496] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has denied Matt Baker <mattbaker at apple.com>'s
request for review:
Bug 189533: Web Inspector: Cookies view should use model objects instead of raw
payload data
https://bugs.webkit.org/show_bug.cgi?id=189533

Attachment 349496: Patch

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




--- Comment #5 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 349496
  --> https://bugs.webkit.org/attachment.cgi?id=349496
Patch

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

r-, only because `WI.Cookie` is missing the "session" value, which is defined
in the protocol.  We should include that, especially if we plan on using model
objects, as otherwise other uses of `cookie.session` won't work.

> Source/WebInspectorUI/UserInterface/Models/Cookie.js:42
> +	   this._type = type;

I've been trying to transition some of our "value bag" style model objects to
use optional parameters for non-required values.

    constructor(type, name, value, {raw, expires, maxAge, path, domain, secure,
httpOnly, sameSite} = {})

This makes it MUCH easier to add/remove parameters and makes the code easier to
read at the construction site (in my opinion).	While you're doing this
refresh, it'd be great if you could do that too (there are only a few creation
sites so it should be easy).

> Source/WebInspectorUI/UserInterface/Models/Cookie.js:86
> +    static fromPayload(payload)

Style: static "things" should be above public "things"