Add Build Timer

Add Build Timer

[sosa, 2010-06-15 22:21:08]

http://codereview.chromium.org/2859005/diff/2001/3001
File build_image (right):

http://codereview.chromium.org/2859005/diff/2001/3001#newcode58
build_image:58: START_TIME=`date +%s`
Use $( date +%s )

http://codereview.chromium.org/2859005/diff/2001/3001#newcode144
build_image:144: set -x
Remove these two lines.  We shouldn't add all those debug information per
builds.

http://codereview.chromium.org/2859005/diff/2001/3001#newcode573
build_image:573: set +e
Why do you need +e here?

http://codereview.chromium.org/2859005/diff/2001/3001#newcode574
build_image:574: END_TIME=`date +%s`
Ditto

http://codereview.chromium.org/2859005/diff/2001/3001#newcode575
build_image:575: elapsed_seconds="$(expr $END_TIME - $START_TIME)"
use $(( END_TIME - START_TIME ))

http://codereview.chromium.org/2859005/diff/2001/3002
File mod_image_for_test.sh (right):

http://codereview.chromium.org/2859005/diff/2001/3002#newcode42
mod_image_for_test.sh:42: START_TIME=`date +%s`
same here

http://codereview.chromium.org/2859005/diff/2001/3002#newcode199
mod_image_for_test.sh:199: set +e
Same comments.

[sosa, 2010-06-15 22:24:31]

It's probably better to just encapsulate this in common.sh i.e. always set
START_TIME in common.sh and create a function report_script_time() that measures
the end time and prints out the stats

[Nick Sanders, 2010-06-15 22:33:55]

Thanks for the feedback, it's definitely cleaner now!

http://codereview.chromium.org/2859005/diff/2001/3001
File build_image (right):

http://codereview.chromium.org/2859005/diff/2001/3001#newcode144
build_image:144: set -x
On 2010/06/15 22:21:08, sosa wrote:
> Remove these two lines.  We shouldn't add all those debug information per
> builds.  

I didn't add them, but I've removed them.

http://codereview.chromium.org/2859005/diff/2001/3001#newcode573
build_image:573: set +e
On 2010/06/15 22:21:08, sosa wrote:
> Why do you need +e here?
expr returns zero, and fails. Removed with $(( ))

http://codereview.chromium.org/2859005/diff/2001/3001#newcode574
build_image:574: END_TIME=`date +%s`
On 2010/06/15 22:21:08, sosa wrote:
> Ditto

Done.

http://codereview.chromium.org/2859005/diff/2001/3001#newcode575
build_image:575: elapsed_seconds="$(expr $END_TIME - $START_TIME)"
On 2010/06/15 22:21:08, sosa wrote:
> use $(( END_TIME - START_TIME ))

Done.

http://codereview.chromium.org/2859005/diff/2001/3002
File mod_image_for_test.sh (right):

http://codereview.chromium.org/2859005/diff/2001/3002#newcode42
mod_image_for_test.sh:42: START_TIME=`date +%s`
On 2010/06/15 22:21:08, sosa wrote:
> same here

Done.

http://codereview.chromium.org/2859005/diff/2001/3002#newcode199
mod_image_for_test.sh:199: set +e
On 2010/06/15 22:21:08, sosa wrote:
> Same comments.

Done.

[sosa, 2010-06-15 22:39:46]

Are you strongly opposed to refactoring into common.sh?

[Nick Sanders, 2010-06-15 22:52:17]

On 2010/06/15 22:39:46, sosa wrote:
> Are you strongly opposed to refactoring into common.sh?

No, I'm doing it now.

[Nick Sanders, 2010-06-15 22:58:52]

Moved to common.sh

[Mandeep Singh Baines, 2010-06-16 03:42:13]

LGTM, Nice. This is much cleaner than before.

[sosa, 2010-06-16 04:09:11]

LGTM!  Thanks for the changes Nick :)

On Tue, Jun 15, 2010 at 8:42 PM,  <msb@chromium.org> wrote:
> LGTM, Nice. This is much cleaner than before.
>
> http://codereview.chromium.org/2859005/show
>