Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1917473002:
Make `git cl description` work for Gerrit (Closed)
|
Created:
4 years, 8 months ago by scottmg Modified:
4 years, 8 months ago Reviewers:
tandrii(chromium) CC:
chromium-reviews, tandrii+omg_git_cl_chromium.org, dpranke+depot_tools_chromium.org, iannucci+depot_tools_chromium.org Base URL:
https://chromium.googlesource.com/chromium/tools/depot_tools.git@master Target Ref:
refs/heads/master Project:
depot_tools Visibility:
Public. |
DescriptionMake `git cl description` work for Gerrit
Example change at https://chromium-review.googlesource.com/c/340430/.
R=tandrii@chromium.org
BUG=603207
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=300188
Patch Set 1 #
Total comments: 2
Patch Set 2 : type #Messages
Total messages: 42 (19 generated)
Description was changed from ========== Make `git cl description` work for Gerrit R=tandrii@chromium.org BUG=603207 ========== to ========== Make `git cl description` work for Gerrit This works in that it actually changes the description R=tandrii@chromium.org BUG=603207 ==========
Description was changed from ========== Make `git cl description` work for Gerrit This works in that it actually changes the description R=tandrii@chromium.org BUG=603207 ========== to ========== Make `git cl description` work for Gerrit This works in that it actually changes the description, but after multiple changes, there's multiple footers added. I'm not sure if they should be stripped in setting the description? Or is the problem in FetchDescription() where it shouldn't be returning 'commit_with_footers', but rather something else? Example change at https://chromium-review.googlesource.com/c/340430/. R=tandrii@chromium.org BUG=603207 ==========
Thanks for CL. I actually didn't do this exactly because I haven't yet figured out what's up with the footers, just like you've written in description. I was considering actually pushing a new patchset with absolutely correct footers. Anyway, this CL LGTM. I think it's better extra footers than no functionality.
The CQ bit was checked by scottmg@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1917473002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1917473002/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Depot Tools Presubmit on tryserver.infra (JOB_FAILED, https://build.chromium.org/p/tryserver.infra/builders/Depot%20Tools%20Presubm...)
The CQ bit was checked by scottmg@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1917473002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1917473002/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Depot Tools Presubmit on tryserver.infra (JOB_FAILED, https://build.chromium.org/p/tryserver.infra/builders/Depot%20Tools%20Presubm...)
Presubmit sez: " ************* Module git_cl W:1989, 0: Method 'GetApprovingReviewers' is abstract in class '_ChangelistCodereviewBase' but is not overridden (abstract-method) " I gather this is a bug in presubmit, not that I lack a proper lgtm? Not sure.
The CQ bit was checked by tandrii@chromium.org
lgtm https://codereview.chromium.org/1917473002/diff/1/gerrit_util.py File gerrit_util.py (right): https://codereview.chromium.org/1917473002/diff/1/gerrit_util.py#newcode550 gerrit_util.py:550: # And the publish it. nit: s/the/then
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1917473002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1917473002/1
https://codereview.chromium.org/1917473002/diff/1/gerrit_util.py File gerrit_util.py (right): https://codereview.chromium.org/1917473002/diff/1/gerrit_util.py#newcode550 gerrit_util.py:550: # And the publish it. On 2016/04/25 19:37:52, tandrii(chromium) wrote: > nit: s/the/then Done.
The CQ bit was checked by scottmg@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tandrii@chromium.org Link to the patchset: https://codereview.chromium.org/1917473002/#ps20001 (title: "type")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1917473002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1917473002/20001
On 2016/04/25 19:37:52, tandrii(chromium) wrote: > lgtm > > https://codereview.chromium.org/1917473002/diff/1/gerrit_util.py > File gerrit_util.py (right): > > https://codereview.chromium.org/1917473002/diff/1/gerrit_util.py#newcode550 > gerrit_util.py:550: # And the publish it. > nit: s/the/then habit to press lgtm isn't a very good one :) ************ Module git_cl W:1989, 0: Method 'GetApprovingReviewers' is abstract in class '_ChangelistCodereviewBase' but is not overridden (abstract-method) " This actually sounds like a valid error, but it has nothing to do with your CL. It should have instead blocked my CLs before, but it didn't. I'm OK if you "git auto-svn && git cl dcommit"
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Depot Tools Presubmit on tryserver.infra (JOB_FAILED, https://build.chromium.org/p/tryserver.infra/builders/Depot%20Tools%20Presubm...)
Description was changed from ========== Make `git cl description` work for Gerrit This works in that it actually changes the description, but after multiple changes, there's multiple footers added. I'm not sure if they should be stripped in setting the description? Or is the problem in FetchDescription() where it shouldn't be returning 'commit_with_footers', but rather something else? Example change at https://chromium-review.googlesource.com/c/340430/. R=tandrii@chromium.org BUG=603207 ========== to ========== Make `git cl description` work for Gerrit This works in that it actually changes the description, but after multiple changes, there's multiple footers added. I'm not sure if they should be stripped in setting the description? Or is the problem in FetchDescription() where it shouldn't be returning 'commit_with_footers', but rather something else? Example change at https://chromium-review.googlesource.com/c/340430/. R=tandrii@chromium.org BUG=603207 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=300169 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as 300169.
Message was sent while issue was closed.
On 2016/04/25 20:10:11, scottmg wrote: > Committed patchset #2 (id:20001) manually as 300169. Hopefully I didn't just break the world by forcing that in. Seems to work on the gerrit side anyway.
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/1922723002/ by hinoka@google.com. The reason for reverting is: Broke presubmit.
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/1922733002/ by hinoka@chromium.org. The reason for reverting is: Broke presubmit.
Message was sent while issue was closed.
Description was changed from ========== Make `git cl description` work for Gerrit This works in that it actually changes the description, but after multiple changes, there's multiple footers added. I'm not sure if they should be stripped in setting the description? Or is the problem in FetchDescription() where it shouldn't be returning 'commit_with_footers', but rather something else? Example change at https://chromium-review.googlesource.com/c/340430/. R=tandrii@chromium.org BUG=603207 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=300169 ========== to ========== Make `git cl description` work for Gerrit This works in that it actually changes the description, but after multiple changes, there's multiple footers added. I'm not sure if they should be stripped in setting the description? Or is the problem in FetchDescription() where it shouldn't be returning 'commit_with_footers', but rather something else? Example change at https://chromium-review.googlesource.com/c/340430/. R=tandrii@chromium.org BUG=603207 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=300169 ==========
Message was sent while issue was closed.
tandrii@chromium.org changed reviewers: - tandrii@chromium.org
Message was sent while issue was closed.
Description was changed from ========== Make `git cl description` work for Gerrit This works in that it actually changes the description, but after multiple changes, there's multiple footers added. I'm not sure if they should be stripped in setting the description? Or is the problem in FetchDescription() where it shouldn't be returning 'commit_with_footers', but rather something else? Example change at https://chromium-review.googlesource.com/c/340430/. R=tandrii@chromium.org BUG=603207 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=300169 ========== to ========== Make `git cl description` work for Gerrit Example change at https://chromium-review.googlesource.com/c/340430/. COMMIT=false R=tandrii@chromium.org BUG=603207 ==========
The CQ bit was checked by tandrii@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
COMMIT=false detected. CQ is abandoning the patch.
Description was changed from ========== Make `git cl description` work for Gerrit Example change at https://chromium-review.googlesource.com/c/340430/. COMMIT=false R=tandrii@chromium.org BUG=603207 ========== to ========== Make `git cl description` work for Gerrit Example change at https://chromium-review.googlesource.com/c/340430/. R=tandrii@chromium.org BUG=603207 ==========
The CQ bit was checked by tandrii@chromium.org
lgtm I've landed a fix for presubmit: https://codereview.chromium.org/1917933003 Now re-landing this CL.
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1917473002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1917473002/20001
Message was sent while issue was closed.
Description was changed from ========== Make `git cl description` work for Gerrit Example change at https://chromium-review.googlesource.com/c/340430/. R=tandrii@chromium.org BUG=603207 ========== to ========== Make `git cl description` work for Gerrit Example change at https://chromium-review.googlesource.com/c/340430/. R=tandrii@chromium.org BUG=603207 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=300188 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as http://src.chromium.org/viewvc/chrome?view=rev&revision=300188
Message was sent while issue was closed.
On 2016/04/26 11:12:28, commit-bot: I haz the power wrote: > Committed patchset #2 (id:20001) as > http://src.chromium.org/viewvc/chrome?view=rev&revision=300188 Thanks for figuring that out and relanding!
Message was sent while issue was closed.
On 2016/04/26 17:15:55, scottmg wrote: > On 2016/04/26 11:12:28, commit-bot: I haz the power wrote: > > Committed patchset #2 (id:20001) as > > http://src.chromium.org/viewvc/chrome?view=rev&revision=300188 > > Thanks for figuring that out and relanding! nah, thanks to you for creating CL :) |
