Re: [PATCH 1/3] fetch-pack: move code to report unmatched refs to a function

2017-02-22 Thread Matt McCutchen
On Wed, 2017-02-22 at 09:11 -0800, Junio C Hamano wrote:
> Matt McCutchen  writes:
> 
> > We're preparing to reuse this code in transport.c for "git fetch".
> > 
> > While I'm here, internationalize the existing error message.
> > ---
> 
> Sounds good.  Please just say it is OK for me to forge your sign-off
> ;-)

Oops.  Given the other issue below, I'll just regenerate the patch
series.

> > diff --git a/fetch-pack.h b/fetch-pack.h
> > index c912e3d..fd4d80e 100644
> > --- a/fetch-pack.h
> > +++ b/fetch-pack.h
> > @@ -45,4 +45,13 @@ struct ref *fetch_pack(struct fetch_pack_args
> > *args,
> >        struct sha1_array *shallow,
> >        char **pack_lockfile);
> >  
> > +/*
> > + * Print an appropriate error message for each sought ref that
> > wasn't
> > + * matched.  Return 0 if all sought refs were matched, otherwise
> > 1.
> > + *
> > + * The type of "sought" should be "const struct ref *const *" but
> > for
> > + * http://stackoverflow.com/questions/5055655/double-pointer-const
> > -correctness-warnings-in-c .
> > + */
> 
> This is an unfinished sentence, but I wonder if we even need to have
> it here?  I'd be surprised if this function was unique in the
> codebase that takes an array pointer whose type is looser than
> necessary because of well-known language rules.

You're probably right.  I'm in the habit of documenting things that
were unknown to me, but I'll take your word for what's well-known to
the average git developer.  I'll remove the remark.

Matt


Re: [PATCH 1/3] fetch-pack: move code to report unmatched refs to a function

2017-02-22 Thread Junio C Hamano
Matt McCutchen  writes:

> We're preparing to reuse this code in transport.c for "git fetch".
>
> While I'm here, internationalize the existing error message.
> ---

Sounds good.  Please just say it is OK for me to forge your sign-off ;-)

> diff --git a/fetch-pack.h b/fetch-pack.h
> index c912e3d..fd4d80e 100644
> --- a/fetch-pack.h
> +++ b/fetch-pack.h
> @@ -45,4 +45,13 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
>  struct sha1_array *shallow,
>  char **pack_lockfile);
>  
> +/*
> + * Print an appropriate error message for each sought ref that wasn't
> + * matched.  Return 0 if all sought refs were matched, otherwise 1.
> + *
> + * The type of "sought" should be "const struct ref *const *" but for
> + * 
> http://stackoverflow.com/questions/5055655/double-pointer-const-correctness-warnings-in-c
>  .
> + */

This is an unfinished sentence, but I wonder if we even need to have
it here?  I'd be surprised if this function was unique in the
codebase that takes an array pointer whose type is looser than
necessary because of well-known language rules.