[webkit-reviews] review granted: [Bug 31442] Chromium Build Slaves : [Attachment 43120] buildbot configuration changes

[bugzilla-daemon at webkit.org]

Mark Rowe (bdash) <mrowe at apple.com> has granted Yaar Schnitman
<yaar at chromium.org>'s request for review:
Bug 31442: Chromium Build Slaves
https://bugs.webkit.org/show_bug.cgi?id=31442

Attachment 43120: buildbot configuration changes
https://bugs.webkit.org/attachment.cgi?id=43120&action=review

------- Additional Comments from Mark Rowe (bdash) <mrowe at apple.com>

>  class BuildFactory(Factory):
> -    def __init__(self, platform, configuration, architectures, triggers):
> +    def __init__(self, platform, configuration, architectures,
triggers=None):
>	   Factory.__init__(self, platform, configuration, architectures, True)

>	   self.addStep(CompileWebKit)
> +	   if platform == "chromium":
> +	       # FIXME: Chromium should archive and trigger tests too.
> +	       return
>	   self.addStep(ArchiveBuiltProduct)
>	   self.addStep(UploadBuiltProduct)
> -	   self.addStep(trigger.Trigger, schedulerNames=triggers)
> +	   if triggers:
> +	       self.addStep(trigger.Trigger, schedulerNames=triggers)

Since there’s no point in archiving or uploading the built product unless
you’re going to do something that uses it, this would make more sense if it
were expressed as:

self.addStep(CompileWebKit)
if triggers:
    self.addStep(ArchiveBuiltProduct)
    self.addStep(UploadBuiltProduct)
    self.addStep(trigger.Trigger, schedulerNames=triggers)

It also removes logic that special-cases Chromium.

I’m going to say r=me but this should be fixed before it is landed.