Re: [PATCHv12 00/23] git notes

[Johan Herland <johan@herland.net>]

On Wednesday 27 January 2010, Junio C Hamano wrote:
> Johan Herland <johan@herland.net> writes:
> > - Patch #23 is a new patch adding the "git notes add" command for
> > appending contents to notes (instead of editing/replacing).
> 
> I find this even more confusing.  Originally I was puzzled by the lack of
> "git notes add"; it took me for quite until I managed to figure out that
> "git notes edit" was the command to use, even if I wanted to add notes to
> a commit that I know that does not have any.

Not sure what you're getting at here. For the case where a commit has no 
notes, "git notes add" and "git notes edit" are already _identical_. I was 
only trying to emphasize that when there is an existing note, "git notes 
add" will append to it, whereas "git notes edit" will replace it with your 
edited version. I'm sorry if this was unclear.

> I would expect "git notes edit" to be "edit starting from the existing
>  one (if exists)", and "git notes add" to be "add notes to a commit that
>  lacks one,

Up to here, the current patch does exactly what you expect.

>  complain if it already has notes, and allow --force to replace".

I disagree. I wrote the current semantics with the following use case in 
mind:

"I have just reviewed a commit, and want to add a 'Reviewed-by' tag to the 
commit notes. I don't really care if the commit already has notes, but I 
certainly _don't_ want to delete them when adding my 'Reviewed-by'. 
Furthermore, I want to do this with a simple command, without being thrown 
into an editor."

Now, 'git notes edit -m "Reviewed-by: ..."' will do the job nicely for 
commits that have no notes, but it will discard any existing notes, so it is 
not a good solution in this case.

Instead, the current semantics of "git notes add" _does_ solve this use case 
('git notes add -m "Reviewed-by: ..."' will append to the existing notes, or 
create a new notes object if none exist).

I'm not opposed to changing the semantics if people find them unintuitive, 
but I would want the new semantics to provide for this use case as well.


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net