RE: [PATCH v3 0/8] refactor the filter process code into a reusable module
Show 36 quoted lines
> From: Junio C Hamano [mailto:gitster@pobox.com]
>
> Ben Peart <peartben@gmail.com> writes:
>
> > Ben Peart (8):
> > pkt-line: add packet_writel() and packet_read_line_gently()
> > convert: Update convert to use new packet_writel() function
> > convert: Split start_multi_file_filter into two separate functions
> > convert: Separate generic structures and variables from the filter
> > specific ones
> > convert: Update generic functions to only use generic data structures
> > convert: rename reusable sub-process functions
> > sub-process: move sub-process functions into separate files
> > convert: Update subprocess_read_status to not die on EOF
>
> This presentation is much easier to digest, compared to the large ball of wax
> we saw previously. It highlights the key modification that cmd2process is
> now "subclassed" from subprocess_entry which is a more generic structure
> by embedding the latter at the beginning, and have its user
> start_multi_file_filter_fn() explicitly downcast the latter to the former around
> patches 4/8 and 5/8.
>
> If I were doing this series, I would organize the first two slightly differently,
> namely:
>
> * 1/8 just adds packet_read_line_gently().
>
> * 2/8 moves packet_write_line() from convert.c to pkt-line.c while
> renaming it, with the justification that this function must be
> made more widely available. It would naturally involves
> adjusting existing callers.
>
> because write and read done in your 1/8 are independent and orthogonal
> changes, and doing it that way also avoids needless temporary duplication of
> the same function.
>
I'll go ahead and do it that way.
> I may later have further comments on 3-8/8 after giving them another read,
> but I haven't seen anything questionable in them so far.
>
> Thanks.
Ben Peart <ben.peart@microsoft.com>