Re: [PATCH v2 2/6] push: move code to setup_push_simple()

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: May 31, 2021, 08:03 UTC
Message-ID: <60b498599fbde_24d28208d4@natae.notmuch>
In-Reply-To: <xmqqczt7xzm4.fsf@gitster.g>

Junio C Hamano wrote:

Show 11 quoted lines

> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > In order to avoid doing unnecessary things and simplify it in further
> > patches.
> >
> > The code is copied exactly as-is; no functional changes.
> 
> The title says "move" and I expected to see corresponding deletions,
> but it seems that this (possibly temporarily) duplicates what these
> two functions do, without removing them, which (possibly
> temporarily) risks the two to drift apart.

OK. Copy then.

Show 5 quoted lines

> From a quick scanning of the remainder of the series, it seems that
> 3/6 and 4/6 improve the copied code without changing the behaviour,
> and at the end these two functions remain, so we have duplicated
> logic for these two functions and improvements only live in one of
> the copies (namely, in the setup_push_simple())?

Not quite: setup_push_upstream() was simplified too, in 5/6.

Maybe I can mention that part of the code of the `simple` mode lives in setup_push_upstream() and by copying the code to the right function both can be simplified.

-- 
Felipe Contreras