[PATCH V2] graft: record the user who performed the command in the extras dictionary

[Matt Harbison]

On Sat, 11 Apr 2015 22:00:22 -0400, Gregory Szorc  
<gregory.szorc at gmail.com> wrote:

> On Sat, Apr 11, 2015 at 9:27 PM, Matt Harbison <mharbison72 at gmail.com>
> wrote:
>
>> On Sat, 11 Apr 2015 17:22:16 -0400, Augie Fackler <raf at durin42.com>  
>> wrote:
>>
>>  On Fri, Apr 10, 2015 at 08:30:17PM -0700, Siddharth Agarwal wrote:
>>>
>>>> On 04/10/2015 07:50 PM, Gregory Szorc wrote:
>>>> >
>>>> > I like the intent of this patch but I'm not a fan of "graft-user." I
>>>> > think answering "who was the last person to 'touch' this commit" is
>>>> > useful beyond graft and I could see us doing something similar for
>>>> > other history editing commands (e.g. rebase).
>>>> >
>>>> > Thinking ahead to when we want this metadata exposed to users  
>>>> (think a
>>>> > template keyword), I'd rather we have a single entity, not N, to
>>>> > represent "last touched by user." "last-user?"
>>>>
>>>> Since we appear to be dancing around the obvious suggestion:
>>>>
>>>> "committer"?
>>>>
>>>> /me runs away
>>>>
>>>
>>> I think you're joking, but I was actually going to suggest just that.
>>>
>>
>> I like it too.  The joke part makes me think I should wait for mpm to
>> approve?
>>
>
> The joke part is this is almost exactly what Git does. I concede to not
> suggesting "committer" myself because I didn't want to invite  
> comparisons.
> Thanks, Siddharth!
>
> But I'm with Martin on this - unless this is pressing I'd like to hear  
> how
> the changelog discussion plays out before adding more metadata on
> changesets. Food for thought: push log is yet another piece of "who did
> what" that would be nice to record. But it can't be part of the hash  
> unless
> you want to invalidate hashes at push time.

It's possible that we don't roll anything out until after August, so  
waiting one more cycle isn't horrible.  I'd rather get it right than have  
it now.  If we are making this more generic than just graft, maybe patch  
imports need it too?

I'm not sure what the changelog discussion entails, but one of the  
nuisance things with extras is not decoding the value where appropriate.   
Some (or most?) values are human readable, but transplant_source for  
example isn't.  Sure, there's a hex filter when using -T.  But I couldn't  
figure out a way to conditionally pass only that value through the filter  
when printing the whole list.  (Kept getting parse errors with {ifeq}.)

I'm not too interested in transplant since graft is in core, but I was  
trying to figure out if the graft-date field should be the integer  
timestamp (quicker for querying) or the localtime string (human  
readable).  And we probably need to figure out how to make the timestamp  
value changes not invalidate the hashes for test suite stability (or  
figure out how to make the tests use a fixed timestamp, which would be  
beneficial for tests that currently glob displayed time strings).