Hi,
On Fri, 5 Dec 2025 at 07:37, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Thu, Dec 04, 2025 at 03:33:30PM +0300, Bilal Yavuz wrote:
> > I applied these feedbacks in v5. I wanted to cover all possible cases
> > so I think 0002 might be a bit more complicated than it needs to be.
> >
> > What do you think about the current implementation?
>
> I'm finding that a bit bloated. My own attempt is the attached, which
> is much simpler, returning only array for the lines of @tail and
> @head. I am not sure to see the use-case in favor of enforcing the
> line count for the caller of the new routine based on what's on this
> thread, so I have left that out to simplify the patch.
I liked your version more. My only complaint was removing the
line_count argument but we can easily add it back when we need it.
> v5 had a mistake: slurp_file() on the full diffs should be removed
> once we dump only the tail and head. It is true that 027 would add an
> extra line if regression.diffs is too short when written this way, but
> the information reported is the same, while keeping the code simpler
> in 027. A portion of the comment at the top of the block printing the
> diffs could be removed: with the limitation of lines in place we don't
> bloat the output anymore.
You are right, thank you for handling that.
v6 LGTM.
--
Regards,
Nazir Bilal Yavuz
Microsoft
