New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 603207 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: ----

Blocking:
issue 579160



Sign in to add a comment

Gerrit: git cl description

Project Member Reported by tandrii@chromium.org, Apr 13 2016

Issue description

Add support for updating description remotely.

Current Gerrit has UI for that, but PolyGerrit doesn't have this functionality.
 
Blocking: 579160
Cc: scottmg@chromium.org
I just tried switching to polygerrit=0 temporarily to edit the message. I was surprised to discover that doesn't work (or rather, it works to edit the message in polygerrit=0-world, but when I switch back to polygerrit=1-world, it remains unedited.)
Labels: Pri-1
This is kind of sucky when there's iteration on a patch, the CL description can end up being quite misleading. Any workaround ideas?

Comment 5 by mark@chromium.org, Apr 19 2016

This happened to me once before, and I thought that editing it in polygerrit=0 was effective for polygerrit=1 too. That was probably three or four weeks ago, though.
OK, let me try again with plain-gerrit to confirm.
No such luck. I edited on https://chromium-review.googlesource.com/?polygerrit=0#/c/339565/ (to remove an empty "BUG=" line)

but the previous version is still on polygerrit=1 at https://chromium-review.googlesource.com/c/339565/ .
I think Gerrit stores the new description outside of the patchset until you publish it (didn't you forget to do so?), which creates a new patchset.
Ah, so edit in polygerrit=0, send mail, and then it should show up in polygerrit=1?
Yep :(
Thanks, that works just fine. (I don't really understand when mail gets sent automatically vs. needing "Publish" in Gerrit).

It would be nice to have this in `git cl desc` or in the UI in polygerrit of course too, but that's workable for now.

(Confirmed here https://chromium-review.googlesource.com/c/339263/)
yyanagisawa@ says that after he edited the description using Gerrit UI (https://chrome-internal-review.googlesource.com/#/c/256181/)
these were added automatically.

Reviewed-on: https://chrome-internal-review.googlesource.com/256181
Reviewed-by: Fumitoshi Ukai <ukai@google.com>
Reviewed-by: Shinya Kawanaka <shinyak@google.com>
Status: Available (was: Untriaged)
Re #12:
I have no idea why that is included. Is it bad/good/just unexpected?
Normally that would be added when you submit if project uses a cherry-pick submit strategy, but you aren't, and this isn't a submit.
So, this must be some Gerrit feature or a bug.
Current my understanding is just unexpected.  No other code review system or commit message in chromium has such (instead of showing reviewed by whom, it shows review URL though).  If you think having such lines is better than not having, that could be negotiable.  I feel having Commit-Queue line and Verified line here are too much, though.

However, I think it bad if the message won't be automatically updated.
e.g. if Alice does CodeReview+1 to this change after I edit for example, I expect "Reviewed-by: Alice <alice@wonderland>" line should also be added when I hit "submit" button or the change has been merged by CQ.


..By the way, I do not think it good to only allow to edit commit message in web UI.  People can edit source in their userland, and git commit --amend to update, it should be natural for them to think they can also update commit message in "git commit --ammend".
(To tell the truth, I have submitted the change with wrong comment several times to chromium because of this ;p)
Cc: yyanagisawa@chromium.org
Agree with your first two paragraphs. I think we can convince GoB to change that for a big customer like Chromium, *so long as* we don't require further customization :) I guess we can create "Chromium-cherry-pick".

As for editing message in UI, I just didn't yet get my hands to it. But I will do this before May.
well, unless somebody is willing to figure this out on his/her own :) patches are welcome :)))
I poked at this and by trial and error got to https://codereview.chromium.org/1917473002, which almost works but not quite right (duplicated footer things that Gerrit seems very fond of).
So, the reason for weird footers is this innocuous 'COMMIT_FOOTERS' I specify when fetching commit description. This actually adds Gerrit-specific footers onto existing description: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#commit-footers

scottmg@ I think your patch is actually OK. What should be fixed though is FetchDescription to do a gitiles request instead on a ref.
actually even simpler:
get hash of revision, and then:
https://chromium.googlesource.com/infra/infra/+/<HASH>?format=json

(for above it's https://chromium.googlesource.com/infra/infra/+/ff905e155a7dcf91d8afc3e7972b93a427f69acc?format=json)
https://codereview.chromium.org/1916123002/#ps1 is my fix. Together with scottmg@ CL, this should be sufficient co close this bug :)
Project Member

Comment 24 by bugdroid1@chromium.org, Apr 25 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools.git/+/2d3da63e29c298d689cef8be5c131d228107a767

commit 2d3da63e29c298d689cef8be5c131d228107a767
Author: tandrii@chromium.org <tandrii@chromium.org>
Date: Mon Apr 25 19:23:27 2016

Fetch Gerrit cl description from gitiles.

R=andybons@chromium.org,scottmg@chromium.org
BUG= 603207 ,605563

Review URL: https://codereview.chromium.org/1916123002

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@300161 0039d316-1c4b-4281-b951-d872f2087c98

[modify] https://crrev.com/2d3da63e29c298d689cef8be5c131d228107a767/gerrit_util.py
[modify] https://crrev.com/2d3da63e29c298d689cef8be5c131d228107a767/git_cl.py

Project Member

Comment 25 by bugdroid1@chromium.org, Apr 25 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build.git/+/98c3ec003a5d5fd72edb92796322bc846e594b71

commit 98c3ec003a5d5fd72edb92796322bc846e594b71
Author: recipe-roller@chromium.org <recipe-roller@chromium.org>
Date: Mon Apr 25 19:32:16 2016

Roll recipe dependencies (trivial).

This is an automated CL created by the recipe roller. This CL rolls recipe
changes from upstream projects (e.g. depot_tools) into downstream projects
(e.g. tools/build).


More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug
(or complain)

depot_tools:
  https://crrev.com/2d3da63e29c298d689cef8be5c131d228107a767 Fetch Gerrit cl description from gitiles. (tandrii@chromium.org)

R=scottmg@chromium.org,andybons@chromium.org,tandrii@chromium.org
BUG=605563, 603207 

TBR=martiniss@chromium.org,phajdan.jr@chromium.org

Review URL: https://codereview.chromium.org/1919003002

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/build@300163 0039d316-1c4b-4281-b951-d872f2087c98

[modify] https://crrev.com/98c3ec003a5d5fd72edb92796322bc846e594b71/infra/config/recipes.cfg

Project Member

Comment 26 by bugdroid1@chromium.org, Apr 25 2016

The following revision refers to this bug:
  http://goto.ext.google.com/viewvc/chrome-internal?view=rev&revision=87004

------------------------------------------------------------------
r87004 | recipe-roller@chromium.org | 2016-04-25T19:43:28.982615Z

-----------------------------------------------------------------
Project Member

Comment 27 by bugdroid1@chromium.org, Apr 25 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools.git/+/4d038b43344946851ef713351f467c0ebfc294ad

commit 4d038b43344946851ef713351f467c0ebfc294ad
Author: scottmg@chromium.org <scottmg@chromium.org>
Date: Mon Apr 25 20:10:05 2016

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 

Review URL: https://codereview.chromium.org/1917473002 .

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@300169 0039d316-1c4b-4281-b951-d872f2087c98

[modify] https://crrev.com/4d038b43344946851ef713351f467c0ebfc294ad/gerrit_util.py
[modify] https://crrev.com/4d038b43344946851ef713351f467c0ebfc294ad/git_cl.py

Project Member

Comment 28 by bugdroid1@chromium.org, Apr 25 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build.git/+/9e7734d5f2b987b3caf18e767708d4970e0e6b57

commit 9e7734d5f2b987b3caf18e767708d4970e0e6b57
Author: recipe-roller@chromium.org <recipe-roller@chromium.org>
Date: Mon Apr 25 20:15:45 2016

Roll recipe dependencies (trivial).

This is an automated CL created by the recipe roller. This CL rolls recipe
changes from upstream projects (e.g. depot_tools) into downstream projects
(e.g. tools/build).


More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug
(or complain)

depot_tools:
  https://crrev.com/4d038b43344946851ef713351f467c0ebfc294ad Make `git cl description` work for Gerrit (scottmg@chromium.org)

R=tandrii@chromium.org,scottmg@chromium.org
BUG= 603207 

TBR=martiniss@chromium.org,phajdan.jr@chromium.org

Review URL: https://codereview.chromium.org/1918923004

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/build@300170 0039d316-1c4b-4281-b951-d872f2087c98

[modify] https://crrev.com/9e7734d5f2b987b3caf18e767708d4970e0e6b57/infra/config/recipes.cfg

Status: Fixed (was: Available)
Command line seems to work now. Do we have a separate bug for having a PolyGerrit UI feature for this?
Project Member

Comment 30 by bugdroid1@chromium.org, Apr 25 2016

The following revision refers to this bug:
  http://goto.ext.google.com/viewvc/chrome-internal?view=rev&revision=87007

------------------------------------------------------------------
r87007 | recipe-roller@chromium.org | 2016-04-25T20:41:19.320700Z

-----------------------------------------------------------------
Project Member

Comment 31 by bugdroid1@chromium.org, Apr 25 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools.git/+/56d0de79a7842d29b59b4f52d1202374c15efbe0

commit 56d0de79a7842d29b59b4f52d1202374c15efbe0
Author: hinoka@chromium.org <hinoka@chromium.org>
Date: Mon Apr 25 22:03:05 2016

Revert of Make `git cl description` work for Gerrit (patchset #2 id:20001 of https://codereview.chromium.org/1917473002/ )

Reason for revert:
Broke presubmit

Original issue's description:
> 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

TBR=tandrii@chromium.org,scottmg@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 603207 

Review URL: https://codereview.chromium.org/1922733002

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@300175 0039d316-1c4b-4281-b951-d872f2087c98

[modify] https://crrev.com/56d0de79a7842d29b59b4f52d1202374c15efbe0/gerrit_util.py
[modify] https://crrev.com/56d0de79a7842d29b59b4f52d1202374c15efbe0/git_cl.py

Project Member

Comment 32 by bugdroid1@chromium.org, Apr 25 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build.git/+/d6503d544954e0106ab0bf1f5fc309e549c1fff6

commit d6503d544954e0106ab0bf1f5fc309e549c1fff6
Author: recipe-roller@chromium.org <recipe-roller@chromium.org>
Date: Mon Apr 25 22:17:02 2016

Roll recipe dependencies (trivial).

This is an automated CL created by the recipe roller. This CL rolls recipe
changes from upstream projects (e.g. depot_tools) into downstream projects
(e.g. tools/build).


More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug
(or complain)

depot_tools:
  https://crrev.com/56d0de79a7842d29b59b4f52d1202374c15efbe0 Revert of Make `git cl description` work for Gerrit (patchset #2 id:20001 of https://codereview.chromium.org/1917473002/ ) (hinoka@chromium.org)
  https://crrev.com/3f0dacf095df6f73a2c54a8a9fb35c1c441a2fe1 Gclient: Don't check if repository is clean if --force is passed in (hinoka@chromium.org)

R=tandrii@chromium.org,scottmg@chromium.org,hinoka@chromium.org
BUG=606420, 603207 

TBR=martiniss@chromium.org,phajdan.jr@chromium.org

Review URL: https://codereview.chromium.org/1916013003

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/build@300177 0039d316-1c4b-4281-b951-d872f2087c98

[modify] https://crrev.com/d6503d544954e0106ab0bf1f5fc309e549c1fff6/infra/config/recipes.cfg

Owner: tandrii@chromium.org
Status: Started (was: Fixed)
Project Member

Comment 34 by bugdroid1@chromium.org, Apr 26 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools.git/+/600b49250fae556f777e7c2469045f0482a59042

commit 600b49250fae556f777e7c2469045f0482a59042
Author: tandrii@chromium.org <tandrii@chromium.org>
Date: Tue Apr 26 10:57:52 2016

Add missing method stub for _GerritCodereviewImpl.

This should make presubmit happy for oncoming git cl desc for Gerrit:
https://codereview.chromium.org/1917473002/

BUG= 603207 
TBR=hinoka@chromium.org,scottmg@chromium.org

Review URL: https://codereview.chromium.org/1917933003

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@300186 0039d316-1c4b-4281-b951-d872f2087c98

[modify] https://crrev.com/600b49250fae556f777e7c2469045f0482a59042/git_cl.py

Project Member

Comment 35 by bugdroid1@chromium.org, Apr 26 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools.git/+/6d1266eef66589112e1131fb0658e5269342b2df

commit 6d1266eef66589112e1131fb0658e5269342b2df
Author: scottmg@chromium.org <scottmg@chromium.org>
Date: Tue Apr 26 11:12:26 2016

Make `git cl description` work for Gerrit

Example change at https://chromium-review.googlesource.com/c/340430/.

R=tandrii@chromium.org
BUG= 603207 

Review URL: https://codereview.chromium.org/1917473002

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@300188 0039d316-1c4b-4281-b951-d872f2087c98

[modify] https://crrev.com/6d1266eef66589112e1131fb0658e5269342b2df/gerrit_util.py
[modify] https://crrev.com/6d1266eef66589112e1131fb0658e5269342b2df/git_cl.py

Status: Fixed (was: Started)
Project Member

Comment 37 by bugdroid1@chromium.org, Apr 26 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools.git/+/c767e3f297058230bb75268842d70866b4d04c24

commit c767e3f297058230bb75268842d70866b4d04c24
Author: tandrii@chromium.org <tandrii@chromium.org>
Date: Tue Apr 26 14:28:49 2016

GetChangeDescriptionFromGitiles followup: future todo for no gitiles.

R=andybons@chromium.org
BUG= 603207 , 605563

Review URL: https://codereview.chromium.org/1913913002

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@300194 0039d316-1c4b-4281-b951-d872f2087c98

[modify] https://crrev.com/c767e3f297058230bb75268842d70866b4d04c24/gerrit_util.py

Project Member

Comment 38 by bugdroid1@chromium.org, Apr 26 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build.git/+/a2564d7cecc040d5bf04521f19faec55eaa6eb46

commit a2564d7cecc040d5bf04521f19faec55eaa6eb46
Author: recipe-roller@chromium.org <recipe-roller@chromium.org>
Date: Tue Apr 26 19:10:46 2016

Roll recipe dependencies (trivial).

This is an automated CL created by the recipe roller. This CL rolls recipe
changes from upstream projects (e.g. depot_tools) into downstream projects
(e.g. tools/build).


More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug
(or complain)

depot_tools:
  https://crrev.com/f30e8728539a259717e63776940ee01f632b0737 depot_tools: add infra_paths recipe module for infra-specific paths (phajdan.jr@chromium.org)
  https://crrev.com/aa71406c1c03d0c8c48ecd9f84285a5d7322b3d6 Add Pdfium to list of can-be-patched projects in chromium checkout. (tandrii@chromium.org)
  https://crrev.com/600b49250fae556f777e7c2469045f0482a59042 Add missing method stub for _GerritCodereviewImpl. (tandrii@chromium.org)
  https://crrev.com/6d1266eef66589112e1131fb0658e5269342b2df Make `git cl description` work for Gerrit (scottmg@chromium.org)
  https://crrev.com/cf48206332e34297adaf8bfed76f0798392a6f3f depot_tools: add test_api to infra_paths recipe module (phajdan.jr@chromium.org)
  https://crrev.com/c808c55594dfca00b57ecc3b3d9eaa8f3a15a81a depot_tools: only add mock infra_paths when test data is enabled (phajdan.jr@chromium.org)
  https://crrev.com/c767e3f297058230bb75268842d70866b4d04c24 GetChangeDescriptionFromGitiles followup: future todo for no gitiles. (tandrii@chromium.org)
  https://crrev.com/54e0d26399a6fd493ff6f88ac00021b463ee35ae Revert of depot_tools: add infra_paths recipe module for infra-specific paths (patchset #2 id:20001 of https://codereview.chromium.org/1915463002/ ) (tandrii@chromium.org)

R=iannucci@chromium.org,machenbach@chromium.org,tandrii@chromium.org,hinoka@chromium.org,andybons@chromium.org,scottmg@chromium.org,martiniss@chromium.org,phajdan.jr@chromium.org
BUG= chromium:605919 , 603207 ,605563

TBR=martiniss@chromium.org,phajdan.jr@chromium.org

Review URL: https://codereview.chromium.org/1916373003

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/build@300201 0039d316-1c4b-4281-b951-d872f2087c98

[modify] https://crrev.com/a2564d7cecc040d5bf04521f19faec55eaa6eb46/infra/config/recipes.cfg

Project Member

Comment 39 by bugdroid1@chromium.org, Apr 26 2016

The following revision refers to this bug:
  http://goto.ext.google.com/viewvc/chrome-internal?view=rev&revision=87067

------------------------------------------------------------------
r87067 | recipe-roller@chromium.org | 2016-04-26T19:23:00.271443Z

-----------------------------------------------------------------
Components: Infra>Codereview
Labels: -Infra-Codereview

Sign in to add a comment