On 05/15, Junio C Hamano wrote: > Brandon Williams <bmwill@xxxxxxxxxx> 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
