[webkit-reviews] review granted: [Bug 57891] Rebaseline queue server : [Attachment 89462] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Mihai Parparita
<mihaip at chromium.org>'s request for review:
Bug 57891: Rebaseline queue server
https://bugs.webkit.org/show_bug.cgi?id=57891

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

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=89462&action=review

Seems reasonable from a high level.  I'm hapy to rs=me, this.  If you were
looking for a nitty-gritty review, I'm not very useful as I dont' understand
the full architecture behind the rebaseline queue server (or what it's really
for).

> Tools/RebaselineQueueServer/app.yaml:4
> +application: rebaseline-queue
> +version: 1
> +runtime: python
> +api_version: 1

Do you want to request a newer version of django here?

> Tools/RebaselineQueueServer/handlers/builderqueue.py:66
> +	   for test in tests:
> +	       if test not in current_tests:

Isn't this just a set intersection?

> Tools/RebaselineQueueServer/handlers/builderqueue.py:69
> +	   self.redirect('/builder/%s/queue' % builder_name)

You might want something to generate these URLs.

> Tools/RebaselineQueueServer/handlers/pages.py:37
> +	   builder_names = QueueEntry.get_builder_names()

get_ isn't really webkit style.

> Tools/RebaselineQueueServer/main.py:31
> +use_library('django', '1.2')

I thought it was possible to do this in app.yaml these days.  But I may be
wrong.