Make git.checkout return the commit hash.

Make git.checkout return the commit hash.

R=dnj@chromium.org, hinoka@chromium.org, luqui@chromium.org, martiniss@chromium.org
BUG=

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

[iannucci, 2016-02-12 21:54:08]

PTAL

Question: is returning just the bare hash good? Or should it e.g. return a
dictionary like {".": "hash", "./path/to/submodule": "otherhash"}, etc. ?

[dnj, 2016-02-12 21:54:45]

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

https://codereview.chromium.org/1693993002/diff/1/recipe_modules/git/api.py#n...
recipe_modules/git/api.py:144: Returns: If set_got_revision is True and the
checkout was successful, this
Why not always return it, even if set_got_revision is not True?

[iannucci, 2016-02-12 21:55:11]

On 2016/02/12 21:54:45, dnj wrote:
> https://codereview.chromium.org/1693993002/diff/1/recipe_modules/git/api.py
> File recipe_modules/git/api.py (right):
> 
>
https://codereview.chromium.org/1693993002/diff/1/recipe_modules/git/api.py#n...
> recipe_modules/git/api.py:144: Returns: If set_got_revision is True and the
> checkout was successful, this
> Why not always return it, even if set_got_revision is not True?

Because we have to run an extra step to get it, which is controlled by
set_got_revision

[iannucci, 2016-02-12 21:55:40]

On 2016/02/12 21:55:11, iannucci wrote:
> On 2016/02/12 21:54:45, dnj wrote:
> > https://codereview.chromium.org/1693993002/diff/1/recipe_modules/git/api.py
> > File recipe_modules/git/api.py (right):
> > 
> >
>
https://codereview.chromium.org/1693993002/diff/1/recipe_modules/git/api.py#n...
> > recipe_modules/git/api.py:144: Returns: If set_got_revision is True and the
> > checkout was successful, this
> > Why not always return it, even if set_got_revision is not True?
> 
> Because we have to run an extra step to get it, which is controlled by
> set_got_revision

I could also just remove set_got_revision entirely. I'm not sure why it's an
option anyway.

[dnj, 2016-02-12 21:56:51]

On 2016/02/12 21:55:40, iannucci wrote:
> On 2016/02/12 21:55:11, iannucci wrote:
> > On 2016/02/12 21:54:45, dnj wrote:
> > >
https://codereview.chromium.org/1693993002/diff/1/recipe_modules/git/api.py
> > > File recipe_modules/git/api.py (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1693993002/diff/1/recipe_modules/git/api.py#n...
> > > recipe_modules/git/api.py:144: Returns: If set_got_revision is True and
the
> > > checkout was successful, this
> > > Why not always return it, even if set_got_revision is not True?
> > 
> > Because we have to run an extra step to get it, which is controlled by
> > set_got_revision
> 
> I could also just remove set_got_revision entirely. I'm not sure why it's an
> option anyway.

Not opposed if that wouldn't cause problems.

[iannucci, 2016-02-12 22:01:05]

On 2016/02/12 21:56:51, dnj wrote:
> On 2016/02/12 21:55:40, iannucci wrote:
> > On 2016/02/12 21:55:11, iannucci wrote:
> > > On 2016/02/12 21:54:45, dnj wrote:
> > > >
> https://codereview.chromium.org/1693993002/diff/1/recipe_modules/git/api.py
> > > > File recipe_modules/git/api.py (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/1693993002/diff/1/recipe_modules/git/api.py#n...
> > > > recipe_modules/git/api.py:144: Returns: If set_got_revision is True and
> the
> > > > checkout was successful, this
> > > > Why not always return it, even if set_got_revision is not True?
> > > 
> > > Because we have to run an extra step to get it, which is controlled by
> > > set_got_revision
> > 
> > I could also just remove set_got_revision entirely. I'm not sure why it's an
> > option anyway.
> 
> Not opposed if that wouldn't cause problems.

It probably would :/

Let me take a stab at it

[iannucci, 2016-02-12 22:07:05]

PTAL

[iannucci, 2016-02-12 22:17:11]

PTAL

[dnj, 2016-02-12 22:21:54]

lgtm

[iannucci, 2016-02-12 22:38:33]

The CQ bit was checked by iannucci@chromium.org

[iannucci, 2016-02-12 22:38:37]

The CQ bit was unchecked by iannucci@chromium.org

[commit-bot: I haz the power, 2016-02-12 22:38:39]

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

[iannucci, 2016-02-12 22:57:00]

Downstream impact for build: https://codereview.chromium.org/1693713005

[iannucci, 2016-02-12 23:01:25]

The CQ bit was checked by iannucci@chromium.org

[iannucci, 2016-02-12 23:01:25]

The patchset sent to the CQ was uploaded after l-g-t-m from dnj@chromium.org
Link to the patchset: https://codereview.chromium.org/1693993002/#ps60001
(title: " ")

[commit-bot: I haz the power, 2016-02-12 23:01:30]

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

[commit-bot: I haz the power, 2016-02-12 23:03:38]

Description was changed from

==========
Make git.checkout return the commit hash.

R=dnj@chromium.org, hinoka@chromium.org, luqui@chromium.org,
martiniss@chromium.org
BUG=
==========

to

==========
Make git.checkout return the commit hash.

R=dnj@chromium.org, hinoka@chromium.org, luqui@chromium.org,
martiniss@chromium.org
BUG=

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

[commit-bot: I haz the power, 2016-02-12 23:03:39]

Committed patchset #4 (id:60001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=298774