Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(386)

Issue 592293002: Report a 12-digit abbreviated hash in the "committed" message (Closed)

Created:
6 years, 3 months ago by Mark Mentovai
Modified:
6 years, 2 months ago
Reviewers:
iannucci
CC:
chromium-reviews, Dirk Pranke, cmp-cc_chromium.org, iannucci+depot_tools_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/tools/depot_tools.git@master
Project:
tools
Visibility:
Public.

Description

Report the entire hash in the "committed" message. This concerns the "committed" message posted to Rietveld. Prior to 34504a15bb3e, this used the committer's own core.abbrev. Now it's using a hard-coded 7. Per recent discussions on chromium-dev, 7 is insufficient to uniquely identify commits in large repositories such as ours. If any abbreviation is to be done, a 12-digit minimum should be used, but there's no reason not to use the entire hash. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=292164

Patch Set 1 #

Patch Set 2 : Use the enitre hash #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M git_cl.py View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 13 (2 generated)
Mark Mentovai
6 years, 3 months ago (2014-09-23 02:41:19 UTC) #2
Mark Mentovai
Ping.
6 years, 2 months ago (2014-09-25 22:34:50 UTC) #3
iannucci
On 2014/09/25 22:34:50, Mark Mentovai wrote: > Ping. lgtm, though to be honest, I think ...
6 years, 2 months ago (2014-09-25 23:50:27 UTC) #4
Mark Mentovai
OK, updated to use the entire hash.
6 years, 2 months ago (2014-09-26 00:48:38 UTC) #5
Mark Mentovai
I agree, for what it’s worth. I don’t think our tools should actually abbreviate anywhere.
6 years, 2 months ago (2014-09-26 00:49:04 UTC) #6
iannucci
lgtm
6 years, 2 months ago (2014-09-26 00:52:34 UTC) #7
iannucci
On 2014/09/26 00:52:34, iannucci wrote: > lgtm Oh, though you should probably update the description.
6 years, 2 months ago (2014-09-26 00:52:47 UTC) #8
Mark Mentovai
I thought I did. I left the “subject” alone to not break it into a ...
6 years, 2 months ago (2014-09-26 01:04:05 UTC) #9
iannucci
lgtm
6 years, 2 months ago (2014-09-26 08:28:52 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/592293002/20001
6 years, 2 months ago (2014-09-26 21:45:58 UTC) #12
commit-bot: I haz the power
6 years, 2 months ago (2014-09-26 21:48:20 UTC) #13
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as 292164

Powered by Google App Engine
This is Rietveld 408576698