[webkit-reviews] review denied: [Bug 28873] WebInspector: Migrate Databases tab to InjectedScript / serialized interaction. : [Attachment 38869] patch

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at hatcher.name> has denied Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 28873: WebInspector: Migrate Databases tab to InjectedScript / serialized
interaction.
https://bugs.webkit.org/show_bug.cgi?id=28873

Attachment 38869: patch
https://bugs.webkit.org/attachment.cgi?id=38869&action=review

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>
> +    isDB: function(db)

I assume this will do somthing more interesting later? If not, I prefer the
callers just doing === or ==.

If this will be needed later, I think isDatabase is better.
> +    runWithTableNames: function(callback)

This name confused me when I saw it. I think getTableNames with a callback is
better wording and more consistent with the other callback functions.

> +	   function withTablesCallback(tableNames)

Starting a function with "with" is odd naming. Just tableNameCallback would
work, or somthing more descriptive on what the function does like
accumulateCompletions.

> -    _queryFinished: function(query, tx, result)
> +    _queryFinished: function(query, result)

Is this correct? I thought tx was required here…

> +	   function withTableNames(tableNames)

Again about "with".

> +	   function withTableNames(tableNames)

Ditto.