On 05/15, Junio C Hamano wrote:
> Brandon Williams <bmw...@google.com> 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

Reply via email to