[webkit-reviews] review denied: [Bug 86916] garden-o-matic should not fetch from debug bots if it also knows about the release bots : [Attachment 143112] Patch

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has denied Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 86916: garden-o-matic should not fetch from debug bots if it also knows
about the release bots
https://bugs.webkit.org/show_bug.cgi?id=86916

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

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=143112&action=review


I agree that this is the right architectural direction (i.e. having more of the
logic on the python side).

> Tools/ChangeLog:15
> +	   (.):

FYI: I usually delete this lines since they bloat the changelog description.

>>
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/script
s/checkout.js:104
>> +		'suffixes': suffixes.join(',')
> 
> By the way, another option here is to send the information in the POST body. 
That way we can send all the builder/test/suffix triples and we won't lose the
association between them.  The server can sort out what to do with them.

I think this is the right thing to do. Lets send this information in the POST
body as JSON.

> Tools/Scripts/webkitpy/tool/commands/rebaseline.py:181
> +    argument_names = "TEST_NAMES..."
> +
> +    def __init__(self):
> +	   options = [
> +	       optparse.make_option('--builders', action='store', default=None,

> +		   help='List of buildbots to pull from'),
> +	       optparse.make_option('--platform-to-move-to', action='store',
default=None,
> +		   help='New platform to move existing baselines to'),
> +	       optparse.make_option('--suffixes', action='store', default=None,

> +		   help='List of output types to rebaseline (as file extension
suffixes)'),

As per the above, this would take JSON as it's only argument and --no-optimize
as it's only flag. I suppose we could make a command that does the current
behavior as well, but I don't really see a use-case for this command outside of
supporting garden-o-matic directly.