[Webkit-unassigned] [Bug 65200] CSS Regions build bot should archive and upload output files

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=65200


Adam Roben (:aroben) <aroben at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #102108|review?                     |review-
               Flag|                            |




--- Comment #3 from Adam Roben (:aroben) <aroben at apple.com>  2011-07-27 05:07:51 PST ---
(From update of attachment 102108)
View in context: https://bugs.webkit.org/attachment.cgi?id=102108&action=review

> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:132
> +def determineExtraFeatures(build):

Can this be a member of the UploadBuiltProduct class?

> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:134
> +    if (build.hasProperty("features")):

No need for the parentheses here.

I think using an early return would be clearer:

if not build.hasProperty('features'):
    return ''

Ditto for the len(features) check below.

> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:141
> +

Missing an extra newline here.

> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:623
>          if triggers:
>              self.addStep(ArchiveBuiltProduct)
>              self.addStep(UploadBuiltProduct)
> -            self.addStep(trigger.Trigger, schedulerNames=triggers)
> +            if triggers.count("upload-only") != len(triggers):
> +                self.addStep(trigger.Trigger, schedulerNames=triggers)

Is it OK to be passing "upload-only", which isn't a real trigger, to the schedulerNames parameter? Maybe it would be better to have some other way of specifying in config.json that you want the build uploaded. Then here you could do "if triggers or thatOtherThing:"

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.