[Webkit-unassigned] [Bug 25517] build-webkit script should print build time at end

[bugzilla-daemon at webkit.org]

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


David Kilzer (ddkilzer) <ddkilzer at webkit.org> changed:

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




--- Comment #2 from David Kilzer (ddkilzer) <ddkilzer at webkit.org>  2009-09-01 21:11:52 PDT ---
(From update of attachment 38883)
This is a great first patch!  Some comments below:

> +2009-09-01  Laurent Cerveau  <lcerveau at me.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +         <http://webkit.org/b/25517> build-webkit script should print build time at end
> +
> +        * Scripts/build-webkit:
> +		Added startTime and endTime variable so that the build time is computed and printed as
> +		part of the build message. Proper display formatting (hours, min, secs).

Lines should be indented by exactly 8 spaces.  There appears to be 9 spaces or
2 tabs used above.

> +my $endTime = time;
> +my $buildTime = $endTime - $startTime;
> +
> +my $buildHours = ($buildTime - $buildTime % 3600)/3600;
> +my $buildMins = (($buildTime - $buildHours*3600) - ($buildTime - $buildHours*3600) %60)/60;
> +my $buildSecs = $buildTime - $buildHours*3600 - $buildMins*60;

This logic would be best in it's own function that takes ($endTime -
$startTime) as input and returns a formatted string.  Maybe call it
formatBuildTime().

I would also leave off hour calculations and only calculate minutes and
seconds.  Most modern hardware builds WebCore in much less than an hour.

Please add spaces before and after all operators (*, %, /, +, -).

> +
> +

There should only be one blank line here.

>  print "\n";
>  print "===========================================================\n";
> -print " WebKit is now built. To run $launcherName with this newly-built\n";
> -print " code, use the \"$launcherPath\" script.\n";
> +print " WebKit is now built ($buildHours:$buildMins:$buildSecs). \n";
> +print " To run $launcherName with this newly-built code, use the\n";
> +print " \"$launcherPath\" script.\n";
>  print "===========================================================\n";

This is fine, but a single $buildTime string from formatBuildTime() would
replace $buildHours:$buildMins:$buildSecs.

r- to fix the above issues.  Thanks!

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