we should only use the commit date for commits in chromium |
||||||
Issue descriptionGit (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).
,
Jun 13 2017
,
Jun 14 2017
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?
,
Jun 14 2017
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).
,
Jun 14 2017
(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.
,
Jun 14 2017
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.
,
Jun 14 2017
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.
,
Jun 14 2017
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 :).
,
Jun 15 2017
,
Jun 15 2017
I’ve begun work on building a per-project option into Gerrit core that will match author and committer dates upon submission.
,
Jun 16 2017
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.
,
Jun 29 2017
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
,
Jun 29 2017
Whoops, linked to the wrong commit. This was the first one: https://chromium.googlesource.com/chromium/src/+/fec87e6b4b1f4ea5463b9eda50a4cbb21c956eec |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by jparent@chromium.org
, Jun 13 2017