On Mon, Jul 9, 2018 at 10:27 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Pratik Karki <predatoramigo@xxxxxxxxx> writes: > > > In the upcoming builtin rebase, we will have to start by detaching > > the HEAD, just like shell script version does. Essentially, we have > > to do the same thing as `git checkout -q <revision>^0 --`, in pure C. > > > > The aforementioned functionality was already present in `sequencer.c` > > in `do_reset()` function. But `do_reset()` performs more than detaching > > the HEAD, and performs action specific to `sequencer.c`. > > > > So this commit refactors out that part from `do_reset()`, and moves it > > to a new function called `detach_head_to()`. As this function has > > nothing to do with the sequencer, and everything to do with what `git > > checkout -q <revision>^0 --` does, we move that function to checkout.c. > > > > This refactoring actually introduces a slight change in behavior: > > previously, the index was locked before parsing the argument to the > > todo command `reset`, while it now gets locked *after* that, in the > > `detach_head_to()` function. > > > > It does not make a huge difference, and the upside is that this closes > > a few (unlikely) code paths where the index would not be unlocked upon > > error. > > > > Signed-off-by: Pratik Karki <predatoramigo@xxxxxxxxx> > > --- > > Here is a place to say "unchanged since v3" for a change like this > one that changes neither the proposed log message above nor the > patch below to help reviewers who have seen the previous round (they > can stop reading here). Hopefully there are more reviewers than you > who write new code, so spending a bit more time to help them spend > less would be an overall win for the project. Thanks for the review. I'll keep this in mind, the next time.
