Re: [PATCH] Reuse previous annotation when overwriting a tag
On Sat, Nov 03, 2007 at 11:54:38AM +0000, Johannes Schindelin wrote:
Show 13 quoted lines
> > +{
> > + int i;
> > + unsigned long size;
> > + enum object_type type;
> > + char *buf, *sp, *eol;
> > + size_t len;
> > +
> > + sp = buf = read_sha1_file(sha1, &type, &size);
> > + if (!buf)
> > + return;
> > + if (!size || (type != OBJ_TAG)) {
>
> Please lose the extra parents.> This can be done much easier with 'sp = strstr(buf, "\n\n");'. You can
> even do that before the previous if(), to free() && return if there is no
> body.
> This can be done much easier with 'eob = strstr(sp, "\n" PGP_SIGNATURE
> "\n");'.
I must say I just stole most of it in show_reference() in the same file.
Show 8 quoted lines
> > +}
> > +
> > static void create_tag(const unsigned char *object, const char *tag,
> > struct strbuf *buf, int message, int sign,
> > - unsigned char *result)
> > + unsigned char *prev, unsigned char *result)
>
> This changes indentation.
Show 11 quoted lines
> > @@ -282,6 +315,10 @@ static void create_tag(const unsigned char *object, const char *tag,
> > if (fd < 0)
> > die("could not create file '%s': %s",
> > path, strerror(errno));
> > +
> > + if (prev)
> > + write_annotation(fd, prev);
> > +
> > write_or_die(fd, tag_template, strlen(tag_template));
>
> Isn't an "else" missing before the write_or_die() here?You're obviously right.
(...)
> Why not teach write_annotations() (or write_tag_body() like I would prefer
> it to be called) to grok a null_sha1? It's not like we care for
> performance here, but rather for readability and ease of use.
I would have if I had looked up for is_null_sha1() earlier ;)
Cheers,
Mike
Mike Hommey <mh@glandium.org>