Re: [WIP/PATCH v4 6/8] for-each-ref: rename some functions and make them public
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- May 31, 2015, 15:30 UTC
- Message-ID
- <556B2921.2000407@gmail.com>
- In-Reply-To
- <CAP8UFD117bJotFQSGjGnSub_0Xq=3wH2o3Du4c49xvmMxAyFDQ@mail.gmail.com>
On 05/31/2015 07:33 PM, Christian Couder wrote:
Show 51 quoted lines
> On Sun, May 31, 2015 at 11:17 AM, Karthik Nayak <karthik.188@gmail.com> wrote: > > On 05/31/2015 01:41 PM, Christian Couder wrote: > >> > >> On Sun, May 31, 2015 at 10:04 AM, Christian Couder > >> <christian.couder@gmail.com> wrote: > >>> > >>> On Sat, May 30, 2015 at 7:53 PM, Karthik Nayak <karthik.188@gmail.com> > >>> wrote: > >>>> > >>>> > >>>> -static void sort_refs(struct ref_sort *sort, struct ref_array *array) > >>>> +void sort_ref_array(struct ref_sort *sort, struct ref_array *array) > >>> > >>> > >>> It is probably better to call the above function ref_array_sort()... > >>> > >>> [...] > >>> > >>>> -static struct ref_sort *default_sort(void) > >>>> +/* If no sorting option is given, use refname to sort as default */ > >>>> +struct ref_sort *ref_default_sort(void) > >>> > >>> > >>> ... especially since you call the above ref_default_sort()... > >>> > >>>> -static int opt_parse_sort(const struct option *opt, const char *arg, > >>>> int unset) > >>>> +int opt_parse_ref_sort(const struct option *opt, const char *arg, int > >>>> unset) > >>> > >>> > >>> ... and the above opt_parse_sort(). > >> > >> > >> After saying that I realize that these two other functions are not > >> doing the same thing. > >> This might suggest that they are not named very well as well. > >> > > > > What do you mean by "not doing the same thing." > > sort_ref_array() is actually sorting a ref_array, while > ref_default_sort() and opt_parse_ref_sort() are not sorting anything. > > Maybe it would all be clearer with a renaming like the following: > > sort_refs() -> ref_array_sort() > struct ref_sort -> struct ref_sort_criteria > default_sort() -> ref_default_sort_criteria() > opt_parse_sort() -> opt_parse_ref_sort_criteria() >
Thanks will follow this, but will change opt_parse_ref_sort_criteria() to parse_opt_ref_sort_criteria() as this is what is commonly followed.
-- Regards, Karthik