From: Mike Hommey <mh@glandium.org>
Date: Sat, 03 Nov 2007 19:55:58 GMT
Subject: Re: [PATCH 1/2] Reuse previous annotation when overwriting a tag
Message-ID: <20071103195558.GA22448@glandium.org>
In-Reply-To: <7vlk9fxj1r.fsf@gitster.siamese.dyndns.org>

On Sat, Nov 03, 2007 at 11:47:44AM -0700, Junio C Hamano wrote:
(...)
> But in this case, as the variable "sp" is never used before it
> is reassigned, I can easily say "drop the useless assignment to
> sp there". ;-)

You got me here ;)

(...)
> When prev is not NULL but points at a null_sha1 nobody writes
> anything out.  Is this intended?
> 
>         In fact, the calling site always passes prev which is
>         prev[] in cmd_tag() and cannot be non-NULL.
 
Damn, I overlooked this, and since the test suite doesn't do anything
on that, that got through. Indeed either the test can be removed, since
write_tag_body does the is_null_sha1() test, or the is_null_sha1() test
can be moved here.

> Why is there "else" in the first place?  Even if you start with
> the previous tag's message, you are launching the editor for the
> user to further edit it, and you would want to give some
> instructions, wouldn't you?

Well, it could be true if the text was more verbose than "Write a tag
message". Anyways, as the test is now, the text is not going to appear.
:(

I'll fix this and will try to enhance the test suite to catch these
problems.

Mike