LGTM


On 2009/01/22 02:47:44, jasvir wrote:
http://codereview.appspot.com/12479/diff/1/2
File tools/dashboard/dashboard.pl (right):

http://codereview.appspot.com/12479/diff/1/2#newcode109
Line 109: our $SELENIUM = "/opt/svn/selenium.sh";
requireExe $ANT;
On 2009/01/22 00:31:54, MikeSamuel wrote:
> Should this file be part of this CL?
> Maybe make the requireExe line up with the others.

No - its build platform specific and simply calls the selenium farm

http://codereview.appspot.com/12479/diff/1/2#newcode119
Line 119: our $BROWSER_FORMAT =
qr/\bVarZ:([\w\.\-]+)=(\d+(?:\.\d+)?)\b/;
On 2009/01/22 00:31:54, MikeSamuel wrote:
> Is this used?

No.  Removed.

http://codereview.appspot.com/12479/diff/1/2#newcode169
Line 169:
extractSeleniumSummary("$REPORTS_DIR/selenium/TESTS-TestSuites.xml",
\...@status_log);
On 2009/01/22 00:31:54, MikeSamuel wrote:
> wrap to 80 columns here and elsewhere

Done.

http://codereview.appspot.com/12479/diff/1/2#newcode374
Line 374: sub extractSeleniumSummary($$) {
On 2009/01/22 00:31:54, MikeSamuel wrote:
> Can you put in a comment with an example of what you're parsing.

Done.



http://codereview.appspot.com/12479

Reply via email to