From: Duy Nguyen <pclouds@gmail.com>
Date: Wed, 26 Oct 2016 13:52:41 GMT
Subject: Re: [PATCH 27/36] attr: convert to new threadsafe API
Message-ID: <CACsJy8AB-24dDEs6Pa_6pB16T2ZAgw2faOQDEtZ2vU2uuAepHg@mail.gmail.com>
In-Reply-To: <20161022233225.8883-28-sbeller@google.com>

On Sun, Oct 23, 2016 at 6:32 AM, Stefan Beller <sbeller@google.com> wrote:
> This revamps the API of the attr subsystem to be thread safe.
> Before we had the question and its results in one struct type.
> The typical usage of the API was
>
>     static struct git_attr_check *check;
>
>     if (!check)
>         check = git_attr_check_initl("text", NULL);


Two cents. I read the .txt first and my first thought was "is _initl a
typo, shouldn't it be jsut _init"? I know we have this 'l' variant at
least in argv-array, but there we have many ways of adding arguments.
And here it's just "initl", not "init" nor other "initX" variants,
which looks odd. I wonder if the name git_attr_check_init() would do
the job fine, since we don't have different init variants and the
naming convention is not strong enough to tell me "it's multiple
arguments ended with a NULL one" right away. If you're worried about
people forgetting NULL at the end, how about passing an array of
strings, terminated by NULL, instead?

Just thinking out loud. Maybe _initl _is_ a better name.
-- 
Duy