Re: [PATCH 2/2] built-in rebase: reinstate `checkout -q` behavior where appropriate

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 13, 2018, 01:49 UTC
Message-ID: <xmqq1s7pzqzl.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <nycvar.QRO.7.76.6.1811121235120.39@tvgsbejvaqbjf.bet>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 11 quoted lines

>> On the other hand, for a file-scope static that is likely to stay as a
>> non-API function but a local helper, it may not be worth it.
>
> Oh, do you think that `reset_head()` is likely to stay as non-API
> function? I found myself in the need of repeating this tedious
> unpack_trees() dance quite a few times over the past two years, and it is
> *always* daunting because the API is *that* unintuitive.
>
> So I *do* hope that `reset_head()` will actually be moved to reset.[ch]
> eventually, and callsites e.g. in `sequencer.c` will be converted from
> calling `unpack_trees()` to calling `reset_head()` instead.

As long as the public API function is well thought out, of course it is OK. Ideally, builtin/reset.c can detect when it is making a hard reset to HEAD and call that same function. Which may or may not mean you would also want to tell it to record ORIG_HEAD and remove MERGE_HEAD and others), perhaps with another bit in that flag word.