Re: [PATCH v4 02/19] convert: add [async_]convert_to_working_tree_ca() variants

>
> On Wed, Nov 4, 2020 at 9:33 PM Matheus Tavares
> <matheus.bernardino@usp.br> wrote:
> >
> > From: Jeff Hostetler <jeffhost@microsoft.com>
> >
> > Separate the attribute gathering from the actual conversion by adding
> > _ca() variants of the conversion functions. These variants receive a
> > precomputed 'struct conv_attrs', not relying, thus, on a index state.
>
> s/a/an/

> > They will be used in a future patch adding parallel checkout support,
> > for two reasons:
> >
> > - We will already load the conversion attributes in checkout_entry(),
> >   before conversion, to decide whether a path is eligible for parallel
> >   checkout. Therefore, it would be wasteful to load them again later,
> >   for the actual conversion.
> >
> > - The parallel workers will be responsible for reading, converting and
> >   writing blobs to the working tree. They won't have access to the main
> >   process' index state, so they cannot load the attributes. Instead,
> >   they will receive the preloaded ones and call the _ca() variant of
> >   the conversion functions. Furthermore, the attributes machinery is
> >   optimized to handle paths in sequential order, so it's better to leave
> >   it for the main process, anyway.
>
> Well explained.
>
> > Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> > [matheus.bernardino: squash, remove one function definition and reword]
>
> <rant++>I'd rather have "Reworked-by: Matheus Tavares
> <matheus.bernardino@usp.br>" or "Improved-by: Matheus Tavares
> <matheus.bernardino@usp.br>" than lines such as the above
> one.</rant++>
>
> > Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
>
> > @@ -1447,7 +1447,7 @@ void convert_to_git_filter_fd(const struct index_state *istate,
> >         ident_to_git(dst->buf, dst->len, dst, ca.ident);
> >  }
> >
> > -static int convert_to_working_tree_internal(const struct index_state *istate,
> > +static int convert_to_working_tree_internal(const struct conv_attrs *ca,
>
> It is still internal, but for consistency it might be better to add
> "_ca" to the name of this function too, as we now pass it a "ca"
> instead of an "istate".