Re: [PATCH 03/35] refspec: rename struct refspec to struct refspec_item
- Date: Tue, 15 May 2018 11:19:50 -0700
- From: Brandon Williams <bmwill@xxxxxxxxxx>
- Subject: Re: [PATCH 03/35] refspec: rename struct refspec to struct refspec_item
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
I'll go ahead and switch these two patches and fixup the comment and