Re: [PATCH 6/6] builtin/tag: add --format argument for tag -v

From: Santiago Torres <santiago@nyu.edu>
Date: Sep 23, 2016, 14:34 UTC
Message-ID: <20160923143358.cl3he5bg27mfxwoy@LykOS.localdomain>
In-Reply-To: <xmqqeg4b39ye.fsf@gitster.mtv.corp.google.com>

> OK, you said something about for_each_ref() in an earlier commit,
> but what you meant was this one, which takes each_tag_name_fn.

Oh yeah, sorry for the confusion.

Show 5 quoted lines

> 
> The function for_each_tag_name(), the type each_tag_name_fn, and the
> function of that type verify_tag(), are ALL file-scope static in
> this single file, builtin/tag.c.  It seems to me that it is not
> necessary to make the format string global at all.

Oh, ok. I was thinking that this was preferred over changing the signature of those functions. (I drew my conclusion from log.c). I'll take this other road then.

Show 7 quoted lines

> 
> ... 
> 
> There are minor implementation and design issues I spotted, but
> overall I think the feature the series attempts to add may be a good
> thing to have.
>

Thanks for the review! I'll re-roll shortly. -Santiago.

> Thanks.