Subject: Re: [PATCH 03/35] refspec: rename struct refspec to struct refspec_item

From: Brandon Williams <[email-masked]>
Date: Tue, 15 May 2018 11:19:50 -0700

On 05/15, Junio C Hamano wrote:
> Brandon Williams <[email-masked]> writes:
> 
> > In preperation for introducing an abstraction around a collection of
> > refspecs (much like how a 'struct pathspec' is a collection of 'struct
> > pathspec_item's) rename the existing 'struct refspec' to 'struct
> > refspec_item'.
> 
> Makes sense.
> 
> >  /* See TAG_REFSPEC for the string version */
> > -const struct refspec *tag_refspec = &s_tag_refspec;
> > +const struct refspec_item *tag_refspec = &s_tag_refspec;
> >  
> >  /*
> >   * Parses 'refspec' and populates 'rs'.  returns 1 if successful and 0 if the
> >   * refspec is invalid.
> >   */
> > -static int parse_refspec(struct refspec *rs, const char *refspec, int fetch)
> > +static int parse_refspec(struct refspec_item *rs, const char *refspec, int fetch)
> 
> The new comment given to the function in the previous step talks
> in terms of the variable name and not type, so technically it is
> still valid after this change without updating.
> 
> I however wonder if it makes more sense to go one step further and
> rename the "const char *" variable that is a string form of a single
> refspec item to something other than "refspec".  Which would
> invalidate the commit you gave to the function and make it necessary
> to be updated in this step.
> 
> I wonder if the early part of the series becomes easier to read if
> patches 2 and 3 are swapped.  That may result in less code/comment
> churn.

I'll go ahead and switch these two patches and fixup the comment and
function name.

-- 
Brandon Williams