[webkit-reviews] review denied: [Bug 20219] Databases pane doesn't recognize table creation/deletion : [Attachment 53610] Proposed patch.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Apr 17 15:18:38 PDT 2010


Joseph Pecoraro <joepeck at webkit.org> has denied Juan C. Montemayor E
<jmonte03 at cs.tufts.edu>'s request for review:
Bug 20219: Databases pane doesn't recognize table creation/deletion
https://bugs.webkit.org/show_bug.cgi?id=20219

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

------- Additional Comments from Joseph Pecoraro <joepeck at webkit.org>
Looks good. I haven't actually tried it locally yet but it looks simple enough.
Some minor issues that would need to be fixed in order for this to get
accepted.


> +2010-04-17  Juan C. Montemayor  <jmonte03 at cs.tufts.edu>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Bug 20219 - Databases pane doesn't recognize table creation/deletion

> +	   https://bugs.webkit.org/show_bug.cgi?id=20219

Nit: prepare-ChangeLog should not have added the "Bug 20219" part, because the
link itself has that. You can remove that text.


>      _queryFinished: function(query, result)
>      {
>	   var dataGrid =
WebInspector.panels.storage.dataGridForResult(result);
> -	   if (!dataGrid)
> -	       return;
> +	   
> +	   if(dataGrid){
>	   dataGrid.element.addStyleClass("inline");
>	   this._appendQueryResult(query, dataGrid.element);
>	   dataGrid.autoSizeColumns(5);
> +	   }

The code inside the if should be indented (4 spaces) as per the style
guidelines:
http://webkit.org/coding/coding-style.html


>	   if (query.match(/^create /i) || query.match(/^drop table /i))
>	       WebInspector.panels.storage.updateDatabaseTables(this.database);

> +
> +	   if(!dataGrid)
> +	       return;
> +

No need to move this code here, since its the end of the function anyways. The
code can just be deleted.


More information about the webkit-reviews mailing list