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

Issue 603195 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

git cl upload on the second time changes description to include Reviewed-on: XXX

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

Issue description

Example:

https://chromium-review.googlesource.com/#/c/338821/1..2//COMMIT_MSG

While this isn't strictly bad, the diff in commit message is confusing, IMO.
 
So, the way it works is like this:

First time git cl upload create issue, the issue is not yet known, so commit msg can't have it.
Second time git cl upload does this:
  1. checks current description, which means it asks to Gerrit. This is to simulate Rietveld behavior which didn't modify the description.
  2. squashes branch and sets the description obtained in #1.
  3. pushes to gerrit.


So, in #1 Gerrit decides to report a different description from what was originally pushed. Not sure what's the best way forward, but I see these so far:
1. leave as is.
2. use whatever message is currently stored in local squashed ref from first upload. The problem is that sometimes there is none, for example after initial upload was on different machine.
3. ask description from gerrit, but parse it and remove Reviewed-on footer. The problem is that sometimes it could be part of previous commit message.
4. actually fetch the remote ref storing the previously pushed commit (as git cl patch does), and fetch description from it. Sadly, this creates more local garbage in hidden refs, potentially error prone (in case of force pushes), and slower.
Cc: scottmg@chromium.org

Comment 3 by aga...@chromium.org, Apr 27 2016

Components: Infra>Codereview
Labels: -Infra-Codereview
Labels: Pri-2
Components: -Infra>Codereview Infra>Codereview>Gerrit Infra>SDK
Labels: Proj-Gerrit-Migration
Labels: Type-Bug
Owner: tandrii@chromium.org
Status: Fixed (was: Available)
I recall actually fixing this by doing #4, though I don't see in which bug.

Sign in to add a comment