Re: [PATCH 1/3] pull: cleanup autostash check

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Jun 15, 2021, 11:09 UTC
Message-ID: <60c88a502d323_e633208fc@natae.notmuch>
In-Reply-To: <CAPUEspg_MmerWb7h8MyhgcJXbWrJeeSyeJ7z2S6eHgDfRDPKvA@mail.gmail.com>

Carlo Arenas wrote:

Show 14 quoted lines

> On Sat, Jun 12, 2021 at 10:11 PM Felipe Contreras <
> felipe.contreras@gmail.com> wrote:
> 
> > -       autostash = config_autostash;
> >         if (opt_rebase) {
> > +               int autostash = config_autostash;
> >                 if (opt_autostash != -1)
> >                         autostash = opt_autostash;
> >
> 
> since you are reducing the scope of the autostash variable anyway, why
> not refactor it additionally for clarity with (something like):
> 
>   int autostash = (opt_autostash != -1) ? opt_autostash : config_autostash;

Because I had like 15 branches on top of this, and wanted 1) to minimize modifications, and 2) to minimize the chance of the patch being rejected, and this is how the code was before f15e7cf5cc (pull: ff --rebase --autostash works in dirty repo, 2017-06-01), so in theory nobody could object.

But yeah, that version makes sense, and in fact I probably had such cleanup in some branch.

-- 
Felipe Contreras