[webkit-reviews] review granted: [Bug 28400] [Chromium] Add compile-only canary-style Chromium slave to the waterfall : [Attachment 35158] Add canary-style Chromium build slave, v4.

[bugzilla-daemon at webkit.org]

Mark Rowe (bdash) <mrowe at apple.com> has granted Dimitri Glazkov (Google)
<dglazkov at chromium.org>'s request for review:
Bug 28400: [Chromium] Add compile-only canary-style Chromium slave to the
waterfall
https://bugs.webkit.org/show_bug.cgi?id=28400

Attachment 35158: Add canary-style Chromium build slave, v4.
https://bugs.webkit.org/attachment.cgi?id=35158&action=review

------- Additional Comments from Mark Rowe (bdash) <mrowe at apple.com>
> diff --git a/WebKitTools/BuildSlaveSupport/build.webkit.org-config/master.cfg
b/WebKitTools/BuildSlaveSupport/build.webkit.org-config/master.cfg
> index 4d92436..5af0698 100644
> --- a/WebKitTools/BuildSlaveSupport/build.webkit.org-config/master.cfg
> +++ b/WebKitTools/BuildSlaveSupport/build.webkit.org-config/master.cfg
> @@ -14,6 +14,7 @@ from buildbot.status.builder import SUCCESS, FAILURE,
WARNINGS, SKIPPED
>  from twisted.internet import defer
>  
>  import simplejson
> +import re

This should be alphabetical.

> +    def createSummary(self, log):
> +	   scraper = re.compile(r"^________ running
'[^\n]+third_party[/\\]WebKit[^\n]+$\n(?:^[UA]\W+[^\n]+$\n)*^(?:Updated to|At)
revision (\d+)", re.DOTALL | re.MULTILINE)
> +	   revs = scraper.findall(log.getText())
> +	   got_revision = "??" # This matches SVN unknown revision response

These two variable names don't follow our coding style.  "revs" should
presumably be "revisions".  "got_revision" should be "gotRevision"..

> +	   if len(revs):
> +	       got_revision = ("r%s" % revs[-1])

The parens are unnecessary here.

> +    def start(self):
> +	   os = self.getProperty("fullPlatform").split('-')[1]
> +	   command = self.command[:]
> +	   if not self.getProperty("revision"):
> +	       # Buildbot will set revision to None when a build is forced. Use
it
> +	       # as a signal to clobber.
> +	       command.append("--clobber")

This introduces a difference in behavior from the other configurations.  While
it may make sense to add this behavior, we shouldn't be doing it in such a way
that it is specific to a single configuration.

r=me