Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2468263005:
Implement and test git cl try for Gerrit. (Closed)
|
Created:
4 years, 1 month ago by tandrii(chromium) Modified:
4 years, 1 month 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. |
DescriptionImplement and test git cl try for Gerrit.
R=borenet@chromium.org,qyearsley@chromium.org
BUG=599931
Committed: https://chromium.googlesource.com/chromium/tools/depot_tools/+/8c5a353e63ac55ace8f761478f085b3be5e65fc2
Patch Set 1 #Patch Set 2 : fix #Patch Set 3 : Make pylint happy with revision_data. #
Total comments: 6
Patch Set 4 : review #Messages
Total messages: 24 (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: Try jobs failed on following builders: Depot Tools Presubmit on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/324548ceadcebb10)
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: Try jobs failed on following builders: Depot Tools Presubmit on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/3247e2d106e0bc10)
LGTM % pylint issues, but I'm not overly familiar with this code and not an OWNER.
On 2016/11/04 13:43:27, borenet2 wrote: > LGTM % pylint issues, but I'm not overly familiar with this code and not an > OWNER. +machenbach@ Pylint is stupid :( revision_data can't be unset at the end of the loop because that is covered by else: statement which raises exception :(
tandrii@chromium.org changed reviewers: + machenbach@chromium.org
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.
lgtm with comment: https://codereview.chromium.org/2468263005/diff/40001/git_cl.py File git_cl.py (right): https://codereview.chromium.org/2468263005/diff/40001/git_cl.py#newcode470 git_cl.py:470: if options.clobber: Looks like this is double now. https://codereview.chromium.org/2468263005/diff/40001/git_cl.py#newcode2880 git_cl.py:2880: return 'CL %s is closed' % (self.GetIssue()) nit: no parentheses needed https://codereview.chromium.org/2468263005/diff/40001/tests/git_cl_test.py File tests/git_cl_test.py (right): https://codereview.chromium.org/2468263005/diff/40001/tests/git_cl_test.py#ne... tests/git_cl_test.py:2050: 'ref': 'refs/changes/56/123456/7' nit: indent?
https://codereview.chromium.org/2468263005/diff/40001/git_cl.py File git_cl.py (right): https://codereview.chromium.org/2468263005/diff/40001/git_cl.py#newcode470 git_cl.py:470: if options.clobber: On 2016/11/04 14:44:58, machenbach (slow) wrote: > Looks like this is double now. true. https://codereview.chromium.org/2468263005/diff/40001/git_cl.py#newcode2880 git_cl.py:2880: return 'CL %s is closed' % (self.GetIssue()) On 2016/11/04 14:44:58, machenbach (slow) wrote: > nit: no parentheses needed Done. https://codereview.chromium.org/2468263005/diff/40001/tests/git_cl_test.py File tests/git_cl_test.py (right): https://codereview.chromium.org/2468263005/diff/40001/tests/git_cl_test.py#ne... tests/git_cl_test.py:2050: 'ref': 'refs/changes/56/123456/7' On 2016/11/04 14:44:58, machenbach (slow) wrote: > nit: indent? Done.
The CQ bit was checked by tandrii@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from borenet@chromium.org, machenbach@chromium.org Link to the patchset: https://codereview.chromium.org/2468263005/#ps60001 (title: "review")
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 ========== Implement and test git cl try for Gerrit. R=borenet@chromium.org,qyearsley@chromium.org BUG=599931 ========== to ========== Implement and test git cl try for Gerrit. R=borenet@chromium.org,qyearsley@chromium.org BUG=599931 Committed: https://chromium.googlesource.com/chromium/tools/depot_tools/+/8c5a353e63ac55... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/tools/depot_tools/+/8c5a353e63ac55... |
