Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2400713003:
git cl try: refactor to add Gerrit support in the future. (Closed)
|
Created:
4 years, 2 months ago by tandrii(chromium) Modified:
4 years, 2 months ago CC:
chromium-reviews, tandrii+omg_git_cl_chromium.org, dpranke+depot_tools_chromium.org, iannucci+depot_tools_chromium.org Target Ref:
refs/heads/master Project:
depot_tools Visibility:
Public. |
Descriptiongit cl try: refactor to add Gerrit support in the future.
R=nodir@chromium.org,sergiyb@chromium.org
BUG=599931
Committed: https://chromium.googlesource.com/chromium/tools/depot_tools/+/e113dfd7a1d6fad4248b5811d69ec9dbbd9a9af0
Patch Set 1 #Patch Set 2 : rebase #
Total comments: 2
Patch Set 3 : Rename. #Messages
Total messages: 25 (16 generated)
The CQ bit was checked by tandrii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
PTAL
lgtm
The CQ bit was checked by tandrii@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
This CL has an open dependency (Issue 2401943002 Patch 20001). Please resolve the dependency and try again. If you are sure that there is no real dependency, please use one of the options listed in https://goo.gl/9Es4OR to land the CL.
The CQ bit was checked by tandrii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2400713003/diff/20001/git_cl.py File git_cl.py (right): https://codereview.chromium.org/2400713003/diff/20001/git_cl.py#newcode1698 git_cl.py:1698: """Returns reason (str) why not, otherwise returns none.""" this makes calling code non-intuitive if x.CanTriggerTryJob(): # actually cannot
https://codereview.chromium.org/2400713003/diff/20001/git_cl.py File git_cl.py (right): https://codereview.chromium.org/2400713003/diff/20001/git_cl.py#newcode1698 git_cl.py:1698: """Returns reason (str) why not, otherwise returns none.""" On 2016/10/11 16:25:49, nodir wrote: > this makes calling code non-intuitive > > if x.CanTriggerTryJob(): > # actually cannot True. Any better name? WhyCantTriggerTryjob? Or maybe: def EnsureCanTriggerTryJob(): ... raise CannotTriggerTryjobError(why) try: cl.EnsureCanTriggerTryJob() except CannotTriggerTryjobError as e: print('can trigger because %s' % e)
On 2016/10/11 16:30:40, tandrii(chromium) wrote: > https://codereview.chromium.org/2400713003/diff/20001/git_cl.py > File git_cl.py (right): > > https://codereview.chromium.org/2400713003/diff/20001/git_cl.py#newcode1698 > git_cl.py:1698: """Returns reason (str) why not, otherwise returns none.""" > On 2016/10/11 16:25:49, nodir wrote: > > this makes calling code non-intuitive > > > > if x.CanTriggerTryJob(): > > # actually cannot > > True. Any better name? WhyCantTriggerTryjob? > Or maybe: > def EnsureCanTriggerTryJob(): > ... > raise CannotTriggerTryjobError(why) > > > try: > cl.EnsureCanTriggerTryJob() > except CannotTriggerTryjobError as e: > print('can trigger because %s' % e) I am not a fan of negative names, but it might be a less evil here, e.g. `if x.CannotTrigerTryJob()` the rest of CL lgtm
The CQ bit was checked by tandrii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Renamed.
The CQ bit was unchecked by tandrii@chromium.org
The CQ bit was checked by tandrii@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sergiyb@chromium.org, nodir@chromium.org Link to the patchset: https://codereview.chromium.org/2400713003/#ps40001 (title: "Rename.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== git cl try: refactor to add Gerrit support in the future. R=nodir@chromium.org,sergiyb@chromium.org BUG=599931 ========== to ========== git cl try: refactor to add Gerrit support in the future. R=nodir@chromium.org,sergiyb@chromium.org BUG=599931 Committed: https://chromium.googlesource.com/chromium/tools/depot_tools/+/e113dfd7a1d6fa... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/tools/depot_tools/+/e113dfd7a1d6fa... |
