Add git_cl recipe module.

Add git_cl recipe module.

BUG=607359
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=300372

[martiniss, 2016-04-27 23:26:34]

martiniss@chromium.org changed reviewers:
+ iannucci@chromium.org, tandrii@chromium.org

[martiniss, 2016-04-27 23:26:35]

PTAL

[tandrii(chromium), 2016-04-28 05:19:35]

where is example.py?

https://codereview.chromium.org/1925873002/diff/1/recipe_modules/git_cl/api.py
File recipe_modules/git_cl/api.py (right):

https://codereview.chromium.org/1925873002/diff/1/recipe_modules/git_cl/api.p...
recipe_modules/git_cl/api.py:8: def __call__(self, name, args, **kwargs):
how about (self, subcmd, args, name='git_cl ' + subcmd, **kwargs) signature?
using would be nicer:
self('description', ['flags and options'])

https://codereview.chromium.org/1925873002/diff/1/recipe_modules/git_cl/api.p...
recipe_modules/git_cl/api.py:9: return self.m.step(name, [
nit: hate to write this, but alignment is weird. Move [ to next line.

[tandrii(chromium), 2016-04-28 05:19:36]

where is example.py?

[iannucci, 2016-04-29 18:55:20]

https://codereview.chromium.org/1925873002/diff/20001/recipe_modules/git_cl/a...
File recipe_modules/git_cl/api.py (right):

https://codereview.chromium.org/1925873002/diff/20001/recipe_modules/git_cl/a...
recipe_modules/git_cl/api.py:12: kwargs['cwd'] = (self.c and
self.c.repo_location) or None
why not api.path['checkout'] ?

https://codereview.chromium.org/1925873002/diff/20001/recipe_modules/git_cl/c...
File recipe_modules/git_cl/config.py (right):

https://codereview.chromium.org/1925873002/diff/20001/recipe_modules/git_cl/c...
recipe_modules/git_cl/config.py:11: def BaseConfig(**_kwargs):
not convinced this is necessary (or desirable)

If you don't like api.path['checkout'] for some reason, I would probably even
prefer

  with api.step.context(cwd="something"):
    # steps

  (Would require adding cwd to context, but that's a very easy change).

Or even

  with api.path.cwd(some_path):
    # api.path['cwd'] == some_path
    # api.step would use `api.path['cwd']` as its cwd value.

[tandrii(chromium), 2016-04-29 19:04:43]

https://codereview.chromium.org/1925873002/diff/20001/recipe_modules/git_cl/c...
File recipe_modules/git_cl/config.py (right):

https://codereview.chromium.org/1925873002/diff/20001/recipe_modules/git_cl/c...
recipe_modules/git_cl/config.py:11: def BaseConfig(**_kwargs):
On 2016/04/29 18:55:20, iannucci wrote:
> not convinced this is necessary (or desirable)
> 
> If you don't like api.path['checkout'] for some reason, I would probably even
> prefer
> 
>   with api.step.context(cwd="something"):
>     # steps
> 
>   (Would require adding cwd to context, but that's a very easy change).
> 
> Or even
> 
>   with api.path.cwd(some_path):
>     # api.path['cwd'] == some_path
>     # api.step would use `api.path['cwd']` as its cwd value.
> 

+1. It's not desirable. path['checkout'] would be sane default, but I really
love context idea - I actually already have it, so martiniss@ how you about you
land without it, and i'll add my patch on top.

https://codereview.chromium.org/1925873002/diff/20001/tests/git_cl_test.py
File tests/git_cl_test.py (right):

https://codereview.chromium.org/1925873002/diff/20001/tests/git_cl_test.py#ne...
tests/git_cl_test.py:6: """Unit tests for git_cl.py."""
i think that's another CL.

[martiniss, 2016-04-29 19:26:48]

https://codereview.chromium.org/1925873002/diff/20001/recipe_modules/git_cl/a...
File recipe_modules/git_cl/api.py (right):

https://codereview.chromium.org/1925873002/diff/20001/recipe_modules/git_cl/a...
recipe_modules/git_cl/api.py:12: kwargs['cwd'] = (self.c and
self.c.repo_location) or None
On 2016/04/29 at 18:55:20, iannucci wrote:
> why not api.path['checkout'] ?

If I switch the recipe roller over to this, then it'll be running git cl upload
from multiple checkouts.

https://codereview.chromium.org/1925873002/diff/20001/recipe_modules/git_cl/c...
File recipe_modules/git_cl/config.py (right):

https://codereview.chromium.org/1925873002/diff/20001/recipe_modules/git_cl/c...
recipe_modules/git_cl/config.py:11: def BaseConfig(**_kwargs):
On 2016/04/29 at 19:04:43, tandrii(chromium) wrote:
> On 2016/04/29 18:55:20, iannucci wrote:
> > not convinced this is necessary (or desirable)
> > 
> > If you don't like api.path['checkout'] for some reason, I would probably
even
> > prefer
> > 
> >   with api.step.context(cwd="something"):
> >     # steps
> > 
> >   (Would require adding cwd to context, but that's a very easy change).
> > 
> > Or even
> > 
> >   with api.path.cwd(some_path):
> >     # api.path['cwd'] == some_path
> >     # api.step would use `api.path['cwd']` as its cwd value.
> > 
> 
> +1. It's not desirable. path['checkout'] would be sane default, but I really
love context idea - I actually already have it, so martiniss@ how you about you
land without it, and i'll add my patch on top.

Ok, i'll just keep this as is for now.

https://codereview.chromium.org/1925873002/diff/20001/tests/git_cl_test.py
File tests/git_cl_test.py (right):

https://codereview.chromium.org/1925873002/diff/20001/tests/git_cl_test.py#ne...
tests/git_cl_test.py:6: """Unit tests for git_cl.py."""
On 2016/04/29 at 19:04:43, tandrii(chromium) wrote:
> i think that's another CL.

Yeah, I goofed :)

[iannucci, 2016-04-29 20:12:44]

rslgtm, looking forward to the context change tho

[iannucci, 2016-04-29 20:12:54]

On 2016/04/29 20:12:44, iannucci wrote:
> rslgtm, looking forward to the context change tho

*sigh*... rs-lgtm

[martiniss, 2016-04-29 20:29:20]

The CQ bit was checked by martiniss@chromium.org

[commit-bot: I haz the power, 2016-04-29 20:29:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1925873002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1925873002/40001

[martiniss, 2016-04-29 20:31:13]

The CQ bit was unchecked by martiniss@chromium.org

[martiniss, 2016-04-29 20:35:39]

The CQ bit was checked by martiniss@chromium.org

[commit-bot: I haz the power, 2016-04-29 20:35:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1925873002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1925873002/40001

[commit-bot: I haz the power, 2016-04-29 20:38:42]

Description was changed from

==========
Add git_cl recipe module.

BUG=607359
==========

to

==========
Add git_cl recipe module.

BUG=607359

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=300372
==========

[commit-bot: I haz the power, 2016-04-29 20:38:43]

Committed patchset #3 (id:40001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=300372

[tandrii(chromium), 2016-05-02 14:14:48]

https://codereview.chromium.org/1925873002/diff/20001/recipe_modules/git_cl/c...
File recipe_modules/git_cl/config.py (right):

https://codereview.chromium.org/1925873002/diff/20001/recipe_modules/git_cl/c...
recipe_modules/git_cl/config.py:11: def BaseConfig(**_kwargs):
On 2016/04/29 19:26:48, martiniss wrote:
> On 2016/04/29 at 19:04:43, tandrii(chromium) wrote:
> > On 2016/04/29 18:55:20, iannucci wrote:
> > > not convinced this is necessary (or desirable)
> > > 
> > > If you don't like api.path['checkout'] for some reason, I would probably
> even
> > > prefer
> > > 
> > >   with api.step.context(cwd="something"):
> > >     # steps
> > > 
> > >   (Would require adding cwd to context, but that's a very easy change).
> > > 
> > > Or even
> > > 
> > >   with api.path.cwd(some_path):
> > >     # api.path['cwd'] == some_path
> > >     # api.step would use `api.path['cwd']` as its cwd value.
> > > 
> > 
> > +1. It's not desirable. path['checkout'] would be sane default, but I really
> love context idea - I actually already have it, so martiniss@ how you about
you
> land without it, and i'll add my patch on top.

OOps, I re-invented the wheel. Now I've re-read what Robbie wrote. The

with api.path.cwd(repo_path):
  ...

makes total sense to me and I have nothing else to offer. Stephen, why does this
not work for you?