On Sat, Dec 5, 2020 at 7:40 AM Christian Couder
<christian.couder@xxxxxxxxx> wrote:
>
> On Wed, Nov 4, 2020 at 9:33 PM Matheus Tavares
> <matheus.bernardino@xxxxxx> wrote:
> >
> > From: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
> >
> > Move convert_attrs() declaration from convert.c to convert.h, together
> > with the conv_attrs struct and the crlf_action enum. This function and
> > the data structures will be used outside convert.c in the upcoming
> > parallel checkout implementation. Note that crlf_action is renamed to
> > convert_crlf_action, which is more appropriate for the global namespace.
>
> It annoys me a bit that some things are called "conv_*" and others
> "convert_*". Maybe we could standardize everything, but it could be a
> separate patch series.
>
> > Signed-off-by: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
> > [matheus.bernardino: squash and reword msg]
>
> Not sure we want the above line, which could actually not be
> completely true if you rework the patch before it gets merged.
Right, I'll remove this line from this and the next patches. Thanks.
> > Signed-off-by: Matheus Tavares <matheus.bernardino@xxxxxx>
>
> > --- a/convert.h
> > +++ b/convert.h
> > @@ -63,6 +63,30 @@ struct checkout_metadata {
> > struct object_id blob;
> > };
> >
> > +enum convert_crlf_action {
> > + CRLF_UNDEFINED,
> > + CRLF_BINARY,
> > + CRLF_TEXT,
> > + CRLF_TEXT_INPUT,
> > + CRLF_TEXT_CRLF,
> > + CRLF_AUTO,
> > + CRLF_AUTO_INPUT,
> > + CRLF_AUTO_CRLF
> > +};
>
> Maybe we should also prepend "CONVERT_" to the values?
Yeah, I also wondered about that. But I wasn't sure if it was worth
the change since there are about 52 occurrences of them. Junio later
mentioned [1] that it might be OK to leave them as-is since the use
sites will always pass these values to the API functions, which would
make it clear that they are from the "convert_" family.
[1]: https://lore.kernel.org/git/xmqqd00z397m.fsf@xxxxxxxxxxxxxxxxxxxxxx/
