Subject: Re: [PATCH v4 02/26] refs: make repack_without_refs and is_branch public

From: David Turner <[email-masked]>
Date: Mon, 19 Oct 2015 19:16:36 -0400

On Fri, 2015-10-16 at 08:34 +0200, Michael Haggerty wrote:
> On 10/15/2015 09:46 PM, David Turner wrote:
> > is_branch was already non-static, but this patch declares it in the
> > header.
> 
> The commit message no longer reflects the patch.
> 
> > Signed-off-by: Ronnie Sahlberg <[email-masked]>
> > Signed-off-by: David Turner <[email-masked]>
> > Signed-off-by: Junio C Hamano <[email-masked]>
> > ---
> >  refs.c | 5 +++--
> >  refs.h | 2 ++
> >  2 files changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/refs.c b/refs.c
> > index fe71ea0..84abc82 100644
> > --- a/refs.c
> > +++ b/refs.c
> > @@ -2816,8 +2816,9 @@ int pack_refs(unsigned int flags)
> >  
> >  /*
> >   * Rewrite the packed-refs file, omitting any refs listed in
> > - * 'refnames'. On error, leave packed-refs unchanged, write an error
> > - * message to 'err', and return a nonzero value.
> > + * 'refnames'. On error, packed-refs will be unchanged, the return
> > + * value is nonzero, and a message about the error is written to the
> > + * 'err' strbuf.
> 
> ^^^ ?
> 
> It is preferable for docstrings to be written in imperative form, so in
> my opinion this is a step backwards...
> 
> ...literally. Your "new" version comes from an older version of Git; it
> was changed in
> 
>     79e4d8a9b8 repack_without_refs(): make function private (2015-06-22)
> 
> to the imperative form.
> 
> Assuming you are using `git-format-patch` to prepare your patches, it is
> always a good idea to read over the prepared email files before sending
> them to the ML, to check for bloopers like this.

Sorry about that one.  It's hard to keep track of what all of these
patches do -- especially the ones that were rebases of Ronnie's.  I've
fixed that and 03/26 as well.

Do you have comments on any of the rest before I re-roll?