git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mike Hommey <mh@glandium.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/2] Reuse previous annotation when overwriting a tag
Date: Sat, 3 Nov 2007 20:55:58 +0100	[thread overview]
Message-ID: <20071103195558.GA22448@glandium.org> (raw)
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

  reply	other threads:[~2007-11-03 19:57 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-03  9:31 [PATCH] Reuse previous annotation when overwriting a tag Mike Hommey
2007-11-03 11:54 ` Johannes Schindelin
2007-11-03 12:10   ` Mike Hommey
2007-11-03 12:23     ` Johannes Schindelin
2007-11-03 13:08       ` [PATCH 1/2] " Mike Hommey
2007-11-03 13:08         ` [PATCH 2/2] Small code readability improvement in show_reference() in builtin-tag.c Mike Hommey
2007-11-07 21:51           ` Mike Hommey
2007-11-03 18:47         ` [PATCH 1/2] Reuse previous annotation when overwriting a tag Junio C Hamano
2007-11-03 19:55           ` Mike Hommey [this message]
2007-11-04  0:11           ` Mike Hommey
2007-11-04  0:11             ` [PATCH 2/2] Add tests for git tag Mike Hommey
2007-11-03 12:27   ` [PATCH] Reuse previous annotation when overwriting a tag Mike Hommey
2007-11-03 12:36     ` Johannes Schindelin
2007-11-03 13:10       ` Mike Hommey
2007-11-03 13:22         ` Johannes Schindelin
2007-11-03 13:59           ` Mike Hommey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20071103195558.GA22448@glandium.org \
    --to=mh@glandium.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).