From: Phillip Wood <phillip.wood123@gmail.com>
Date: Fri, 01 Sep 2023 13:33:53 GMT
Subject: Re: [PATCH 1/2] builtin/rebase.c: Emit warning when rebasing without a forkpoint
Message-ID: <4ee8802b-0b54-4ed3-8ead-61e7d7628bce@gmail.com>
In-Reply-To: <xmqq1qfiubg5.fsf@gitster.g>

On 31/08/2023 22:52, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> I am not commenting on the tests, as the above code probably needs
>> to be corrected first so that folks who want to squelch the message
>> and want the "forkpoint behaviour by default when rebuilding on the
>> usual upstream" behaviour can do so by setting the variable to true.
>>
>> And that obviously need to be tested, too.
> 
> Another worrysome thing about rebase.forkpoint is that it will be
> inevitable for folks to start complaining that it does not work the
> way other configuration variables do.  Setting the variable to
> 'true' is not the same as passing '--fork-point=true' from the
> command line.

It does seem strange, it looks like the variable was really added as a 
way to turn off the current default. If we do change the default to 
--no-fork-point when no upstream is given on the commandline then I 
think we should consider allowing "auto" for rebase.forkpoint with the 
some meaning as "true" and recommend that instead.

Best Wishes

Phillip

> I actually think it would be a lot larger behaviour change with a
> huge potential to be received as a regression if we start making the
> variable to mean the same thing as passing '--fork-point=true'.
> People may like the current "if you are rebuilding your branch on
> its usual upstream, pay attention to the rebase and rewind of the
> upstream itself, but if you are giving an explicit upstream from the
> command line, the tool does not second guess you with the fork-point
> heuristics" behaviour and prefer to set it to true.  We would be
> breaking them big time if suddenly the rebase.forkpoint=true they
> set previously starts triggering the fork-point heuristics when they
> run "git rebase upstream".  So that needs to be kept in mind when/if
> we fix the "setting the variable, even to 'true', will squelch the
> warning".
>