Web lists-archives.com

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




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.