[webkit-reviews] review granted: [Bug 207648] [GTK] Add EWS testers to run GTK layout tests : [Attachment 393466] Patch

[bugzilla-daemon at webkit.org]

Carlos Alberto Lopez Perez <clopez at igalia.com> has granted Diego Pino
<dpino at igalia.com>'s request for review:
Bug 207648: [GTK] Add EWS testers to run GTK layout tests
https://bugs.webkit.org/show_bug.cgi?id=207648

Attachment 393466: Patch

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




--- Comment #21 from Carlos Alberto Lopez Perez <clopez at igalia.com> ---
Comment on attachment 393466
  --> https://bugs.webkit.org/attachment.cgi?id=393466
Patch

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

r=me. Patch looks fine.
But I have a couple of suggestions, so its nice if you can take them into
account before landing.

> Tools/ChangeLog:5
> +	   [GTK] Add EWS testers to run GTK layout tests
> +	   https://bugs.webkit.org/show_bug.cgi?id=207648
> +

I miss general comments on your changelogs. Something as simple as:
"The queue GTK-Webkit2-EWS its now renamed to GTK-Build-EWS and we add a new
queue for WK2 Tests (layout tests) named GTK-Webkit2-EWS"
Makes easier understand the goal of the patch for those taking a look at it

> Tools/BuildSlaveSupport/ews-build/config.json:319
> +	 "name": "GTK-Build-EWS",

There was an unit-test testing for this name that it discovered by grepping the
old name on the code.
We should update the test now that the name its gone. Proposed update:
http://sprunge.us/k3EfbI