Re: [PATCH v5 09/11] rebase: support automatic notes copying

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thomas Rast <trast@xxxxxxxxxxxxxxx> writes:

> Luckily, all the support already happens to be there.
>
> Signed-off-by: Thomas Rast <trast@xxxxxxxxxxxxxxx>

I did my first "rebase -i / commit --amend / rebase --continue" cycle with
this series, and it correctly carried notes I keep track of Message-Id of
original patches with.  Good job.

After looking at the output from "git log refs/notes/amlog", I have a few
observations, though.

 - "git notes add" and friends honor GIT_COMMITER_* and GIT_AUTHOR_* as
   usual.  Since I use post-applypatch hook to record the message-id from
   "git am", I ended up getting my notes written by original submitters.

 - The log messages are mostly useless.  The scripted implementation said
   "Annotate 6d8094a8aa32c30ff39c6b8609acb8d057ccb5e5" which was not the
   most informative, but the new one says "Notes added by 'git notes add'".

I haven't thought things through thoroughly, and I am not complaining, but
it may be worth thinking about what the "commit"-ness of generations of
notes tree _should_ mean.

As we are recording the history of how the set of my notes have grown over
time, I think it is natural to expect that the user can record who is
recording this notes tree at what time for what reason, just like a
regular commit does.

 - "added by 'git notes add'" is just as immaterial as "commit log edited
   with vi".  'copied via notes.rebase.rewrite hook' would make much more
   sense.

 - "git notes add" already uses -m option to take the text used as note
   (i.e. payload of the commit); the user may need a way to affect the
   commit log message to record why the note is added (or modified).

 - As to the determination of committer/author identity, I think what the
   code currently does (i.e. honor the environment and user.* config) is
   perfectly sane, but at the same time I think it would be surprising for
   unsuspecting users.  We may want to advise users about this in the
   documentation.

But the above is merely my personal feeling on the "notes history".

The notes implementation (from the scripted one to the one in 'pu') seems
to take quite a different view and is designed as if aspects other than
the topology and the tree each "commit" object records are not useful at
all, and commits are used to implement the notes history only because the
ancestry might help when we later implement merges of notes histories.  I
think that could also be a _valid_ position to take.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]