Re: [PATCH 5/8] fetch: deduplicate handling of per-reference format

From: Patrick Steinhardt <ps@pks.im>
Date: Mar 17, 2023, 09:51 UTC
Message-ID: <ZBQ4Cqkq9v2cB77f@ncase>
In-Reply-To: <xmqqv8j0ljxd.fsf@gitster.g>

On Thu, Mar 16, 2023 at 09:50:38AM -0700, Junio C Hamano wrote:

Show 19 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
> 
> >> So, I dunno.  The result of applying this patch leaves it in an
> >> in-between state, where the division of labor between the caller and
> >> the callee smells somewhat iffy.
> >> 
> >> Thanks.
> >
> > I totally agree with you here. From my point of view this "division of
> > labor" is getting fixed in the final patch that then also moves the
> > printing logic into `format_display()`.
> 
> Yes, again I smell the same "isn't this series going a bit too
> roundabout route to its goal?" which I expressed in my response to
> an earlier step.  The endgame of the series, even though I may not
> agree with it 100%, is self consistent and does not leave the "this
> ends at an in-between state" aftertaste in my mouth.
> 
> Thanks.

Yeah. I did have some problems to lay out this series in a sensible way. In an earlier iteration I tried moving the printing logic in one of the initial patches, but from my point of view that resulted in an even more awkward in-between state where the formatting and printing logic was kind of all over the place. And another try with "big-bang-refactoring" was barely reviewable, either.

Maybe the solution is to keep the order, but document intent better in each of the patches leading towards the unified printing logic.

Patrick