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

Issue 732968 link

Starred by 11 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

we should only use the commit date for commits in chromium

Project Member Reported by dpranke@chromium.org, Jun 13 2017

Issue description

Git (and Gerrit) have the concept of "author", "author date", "committer", and "committer date".

In Rietveld, we used both author and committer, but we would always use the same date (the commit date) for both values. Switching to Gerrit allows us to have "real" Author Dates, but since Git shows author dates by default (in things like git log and git blame), this can be confusing for people who aren't used to or don't want to think that way).

One option is to get used to the author date being the default.

Another is to try and make sure all of our tooling defaults readily to showing commit date instead of author date.

A third is to just throw away the author date and set it to the commit date when the change is submitted, like Rietveld did.

I think there's a general consensus that for most things in Chromium the commit date is much more interesting than the author date, and so the first option isn't particularly compelling, and we should probably do either the second or the third.

I'm filing this bug to track the idea that we should do the third option. A WontFix verdict on this bug would implicitly be a vote for either the first or second option (or, I suppose, some hypothetical fourth option).
 
If we can do (2) reasonably, I think that is the best compromise.

Would it help to audit all known places we expect issues (e.g. git blame) and use that to inform a decision?
Cc: dborowitz@google.com
I would add a little color to the "general consensus" point fwiw: on the chromium-dev@ thread, my analysis is that Chromium "user-land" devs were overwhelmingly in favor of the third option, while infra devs were basically unanimously in favor of the second option. Does that seem accurate?
On option (2) of updating all our tooling to show/use committed date:
  Not all tooling used by chromium devs is our tooling.
  For example, git has a bunch of subcommands which show or use timestamps. 
  Changing these as well doesn't seem feasible,
  and not changing these invites for weird inconsistencies.

More importantly, (3) is a lot easier to do once we are 100% on Gerrit because there will be just 1 way to add commits to chromium - Gerrit CL.
We already have a Git-Numberer plugin verifying and generating correct commit message. It's very easy (for me, at least) to create another plugin to modify commit author timestamp to be current time*. Well, of course if Gerrit team agrees :) 


* current time != commit timestamp in general, but pretty close. One can also extend submit strategy to have exactly the same timestamp, but it is a bit more involved and adds another option to Gerrit's core which has to be exposed to already crowded project config menu (though it can perhaps be made per-Gerrit host).
(3) is already what Rietveld does in my understanding (sort of by accident if I understood Aaron correctly :), so this is a Gerrit-only issue.
Note that there is option 4 as well, which is to add a strategy to Gerrit to actually just do what we want.  This is essentially the same as (3), except that it would be a per project config built into Gerrit, rather than another plugin that we maintain and need Gerrit to provide an API for.  We are discussing this with them as well.
Note that Julie's option 4 is the same thing Andrii described in his asterisk :)

> to modify commit author timestamp to be current time

Nontrivial, unfortunately. The extension point you have today, quite intentionally, only allows you to modify the commit message, not other commit metadata. Gerrit will not add an interface that allows you to arbitrarily modify the commit--we don't want plugins changing parent or tree pointers, for example.

So we could in theory add an extension point that allows you to modify the author, but really, I'm struggling to imagine a hypothetical plugin that would modify the author in any other way other than to tweak the timestamp. At which point just building that into core with an option sounds more appealing than an extension point.
From the naive Gerrit user point of view, it seems more intuitive to me to have this be a different submit strategy than to be something customizable via a plugin API; I agree with dborowitz@ that wanting to do anything other than tweak the timestamp seems like a reach. But my opinion probably isn't worth much in this case :).
Cc: sdefresne@chromium.org
Labels: Proj-Gerrit-Migration
Owner: andyb...@chromium.org
Status: Started (was: Untriaged)
I’ve begun work on building a per-project option into Gerrit core that will match author and committer dates upon submission.
Labels: Milestone-Afterglow
Putting in Milestone-Afterglow to reflect the fact that this work has been deemed non-blocking, but leaving at Pri-1 because this is our highest priority in parallel to launch, Andy is already working on it, and we plan to have it in place ASAP no matter what.
Status: Fixed (was: Started)
The internal changes have been landed and deployed.

The config has been changed: https://chromium.googlesource.com/chromium/src/+/c414cefc476ee5d40f58f88b5680d573e624eed8

And we have our first commit with the author date forged to match the committer date: https://chromium.googlesource.com/chromium/src/+/6ae8ca822914c7216962dbf2255af34aa5605622
Whoops, linked to the wrong commit. This was the first one: https://chromium.googlesource.com/chromium/src/+/fec87e6b4b1f4ea5463b9eda50a4cbb21c956eec

Sign in to add a comment