Add ability to retrieve cl commit bit on cmd line.

Add ability to retrieve cl commit bit on cmd line.

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

[Michael Achenbach, 2015-11-30 13:18:47]

Description was changed from

==========
Add ability to retrieve cl commit bit on cmd line.

BUG=
==========

to

==========
Add ability to retrieve cl commit bit on cmd line.

BUG=chromium:563434
==========

[Michael Achenbach, 2015-11-30 13:18:47]

machenbach@chromium.org changed reviewers:
+ hablich@chromium.org, tandrii@chromium.org

[Michael Achenbach, 2015-11-30 13:20:48]

PTAL

[Michael Hablich, 2015-11-30 13:24:19]

lgtm with nit. Sorry that I have not seen that earlier.

https://codereview.chromium.org/1485663002/diff/20001/commit_queue.py
File commit_queue.py (right):

https://codereview.chromium.org/1485663002/diff/20001/commit_queue.py#newcode85
commit_queue.py:85: if e.code == 404:
Nit: Line 85 to 91 could be extracted in a separate function so there is not
duplicate code.

[tandrii(chromium), 2015-11-30 13:29:06]

LGTM

[tandrii(chromium), 2015-11-30 13:29:38]

<<<<<<-------- and maybe you should switch to git checkout

[Michael Achenbach, 2015-11-30 13:35:37]

How about now?

https://codereview.chromium.org/1485663002/diff/20001/commit_queue.py
File commit_queue.py (right):

https://codereview.chromium.org/1485663002/diff/20001/commit_queue.py#newcode85
commit_queue.py:85: if e.code == 404:
On 2015/11/30 13:24:19, Hablich wrote:
> Nit: Line 85 to 91 could be extracted in a separate function so there is not
> duplicate code.

Done.

[Michael Achenbach, 2015-11-30 13:35:54]

On 2015/11/30 13:29:38, tandrii(chromium) wrote:
> <<<<<<-------- and maybe you should switch to git checkout

For this I have to re-up the CL, right?

[tandrii(chromium), 2015-11-30 13:54:09]

LGTM

and don't bother about SVN: this CL will land through CQ either way. it was that
you probably should upgrade your checkout in general.

https://codereview.chromium.org/1485663002/diff/60001/commit_queue.py
File commit_queue.py (right):

https://codereview.chromium.org/1485663002/diff/60001/commit_queue.py#newcode83
commit_queue.py:83: fun(obj.get_issue_properties(issue, False))
this should be return, IMO, even though this CL would work either way.

[Michael Achenbach, 2015-11-30 13:57:10]

https://codereview.chromium.org/1485663002/diff/60001/commit_queue.py
File commit_queue.py (right):

https://codereview.chromium.org/1485663002/diff/60001/commit_queue.py#newcode83
commit_queue.py:83: fun(obj.get_issue_properties(issue, False))
On 2015/11/30 13:54:09, tandrii(chromium) wrote:
> this should be return, IMO, even though this CL would work either way.

Done. Also explicitly made the functions return 0 now.

[Michael Achenbach, 2015-11-30 13:59:35]

The CQ bit was checked by machenbach@chromium.org

[Michael Achenbach, 2015-11-30 13:59:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
hablich@chromium.org, tandrii@chromium.org
Link to the patchset: https://codereview.chromium.org/1485663002/#ps80001
(title: "Review")

[commit-bot: I haz the power, 2015-11-30 13:59:45]

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

[commit-bot: I haz the power, 2015-11-30 14:01:47]

Description was changed from

==========
Add ability to retrieve cl commit bit on cmd line.

BUG=chromium:563434
==========

to

==========
Add ability to retrieve cl commit bit on cmd line.

BUG=chromium:563434

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

[commit-bot: I haz the power, 2015-11-30 14:01:47]

Committed patchset #5 (id:80001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=297735

[tandrii(chromium), 2015-11-30 14:03:11]

https://codereview.chromium.org/1485663002/diff/80001/commit_queue.py
File commit_queue.py (right):

https://codereview.chromium.org/1485663002/diff/80001/commit_queue.py#newcode80
commit_queue.py:80: def _apply_on_issue(fun, obj, issue):
well, if you are into making re-usable thing, why not make it like this:
  def _get_issue_properties(obj, issue):
    try: ...
    except:
      ...
      # instead of return 1
      sys.exit(1)

and use as nicely as

def get_commit(..):
  print int(_get_issue_properties(obj, issue)['commit'])

[Michael Achenbach, 2015-11-30 14:11:07]

https://codereview.chromium.org/1485663002/diff/80001/commit_queue.py
File commit_queue.py (right):

https://codereview.chromium.org/1485663002/diff/80001/commit_queue.py#newcode80
commit_queue.py:80: def _apply_on_issue(fun, obj, issue):
On 2015/11/30 14:03:11, tandrii(chromium) wrote:
> well, if you are into making re-usable thing, why not make it like this:
>   def _get_issue_properties(obj, issue):
>     try: ...
>     except:
>       ...
>       # instead of return 1
>       sys.exit(1)
> 
> and use as nicely as
> 
> def get_commit(..):
>   print int(_get_issue_properties(obj, issue)['commit'])

I'm not entirely sure about the semantics of the callers. The expect a return
code and calling sys.exit(1) would circumvent this. E.g. the caller might have
some more logic and print additional help message or something...