Add branch support to preflight

Add branch support to preflight

Change-Id: Ia2592c6384e94e4e8e5400eddbf2e03aac150eef

BUG=7905
TEST=run cbuildbot on my workstation.

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=a6b1390

[scottz, 2010-10-21 23:31:07]

http://codereview.chromium.org/4040002/diff/2001/3001
File bin/cbuildbot.py (right):

http://codereview.chromium.org/4040002/diff/2001/3001#newcode233
bin/cbuildbot.py:233: if internal:
per Sosa's previous statement when we were chatting about this, i'd prefer an
option on the CLI and then just pass this in as a variable.

cbuildbot.py --repo ssh:// or http:// etc etc.

http://codereview.chromium.org/4040002/diff/2001/3001#newcode407
bin/cbuildbot.py:407: parser.add_option('-t', '--tracking_branch',
default="cros/master",
single quotes around default.

http://codereview.chromium.org/4040002/diff/2001/3001#newcode408
bin/cbuildbot.py:408: help='Run the buildbot on a branch')
everything else uses a dest. Please add a dest for this as well. I know it
automatically converts your --long-option but lets just keep it consistent. 

Also underscore? how about --tracking-branch and dest='tracking_branch' this is
fairly typical and I prefer that to using an underscore in the cli argument

http://codereview.chromium.org/4040002/diff/2001/3001#newcode411
bin/cbuildbot.py:411: help='Run the buildbot on internal manifest')
As per before, if we just make the repo url a variable here this goes away.

[anush, 2010-10-22 03:15:39]

On 2010/10/21 23:31:07, scottz wrote:
> http://codereview.chromium.org/4040002/diff/2001/3001
> File bin/cbuildbot.py (right):
> 
> http://codereview.chromium.org/4040002/diff/2001/3001#newcode233
> bin/cbuildbot.py:233: if internal:
> per Sosa's previous statement when we were chatting about this, i'd prefer an
> option on the CLI and then just pass this in as a variable.

done

> 
> cbuildbot.py --repo ssh:// or http:// etc etc.
> 
> http://codereview.chromium.org/4040002/diff/2001/3001#newcode407
> bin/cbuildbot.py:407: parser.add_option('-t', '--tracking_branch',
> default="cros/master",
> single quotes around default.

done

> 
> http://codereview.chromium.org/4040002/diff/2001/3001#newcode408
> bin/cbuildbot.py:408: help='Run the buildbot on a branch')
> everything else uses a dest. Please add a dest for this as well. I know it
> automatically converts your --long-option but lets just keep it consistent. 
> 
> Also underscore? how about --tracking-branch and dest='tracking_branch' this
is
> fairly typical and I prefer that to using an underscore in the cli argument

done

> 
> http://codereview.chromium.org/4040002/diff/2001/3001#newcode411
> bin/cbuildbot.py:411: help='Run the buildbot on internal manifest')
> As per before, if we just make the repo url a variable here this goes away.

done

[scottz, 2010-10-22 05:31:22]

LGTM, unfortunate the merge shows up as your changes but meh ;)

[sosa, 2010-10-22 06:20:06]

LGTM w/ nit

http://codereview.chromium.org/4040002/diff/6001/7001
File bin/cbuildbot.py (right):

http://codereview.chromium.org/4040002/diff/6001/7001#newcode262
bin/cbuildbot.py:262: branch = tracking_branch.split('/');
Nit: may wanna consider just using os.path.basename() here

[anush, 2010-10-22 17:15:12]

Missed this comment after scott's LGTM. Will change this when I modify
cbuildbot again.

Thanks

On Thu, Oct 21, 2010 at 11:20 PM, <sosa@chromium.org> wrote:

> LGTM w/ nit
>
>
> http://codereview.chromium.org/4040002/diff/6001/7001
>
> File bin/cbuildbot.py (right):
>
> http://codereview.chromium.org/4040002/diff/6001/7001#newcode262
> bin/cbuildbot.py:262: branch = tracking_branch.split('/');
> Nit: may wanna consider just using os.path.basename() here
>
>
> http://codereview.chromium.org/4040002/show
>