Re: [GSoC][PATCH 2/3] rebase -i: rewrite setup_reflog_action() in C

[Alban Gruin <alban.gruin@gmail.com>]

Hi Phillip,

Le 18/06/2018 à 17:34, Phillip Wood a écrit :
> On 18/06/18 14:18, Alban Gruin wrote:
>> This rewrites setup_reflog_action() from shell to C.
>>
>> A new command is added to rebase--helper.c, “setup-reflog”, as such as a
>> new flag, “verbose”, to silence the output of the checkout operation
>> called by setup_reflog_action().
> 
> I'm having difficulty understanding what that means, surely the verbose
> flag is to stop the output from being silenced.
> 

I reversed the meaning of the flag, I will correct it in a reroll.

> I'm not that keen on the naming in this patch, if it's only a staging
> post and will be removed it probably doesn't matter too much. I can see
> why you've based the function and flag names on the shell version, but
> the C version is not setting up the reflog it is checking out the branch
> we're rebasing. --checkout-base or something like that would be clearer.
> 
> Also the name of the function that does the checkout is tied to checking
> out the base revision, but then reused in the next patch to checkout the
> 'onto' commit. As such I think it would be clearer if it was called
> run_git_checkout() or something like that.
> 

Right.

> One further thought - how easy would it be to refactor the code in
> do_reset() to handle the checkouts in this patch series, rather than
> having to fork 'git checkout'
> 

Good remark, I did not notice do_reset().  I intend to refactor and
optimize after I have done (most of) the conversion, so this goes into
my todo-list, but it does not really seem difficult at first sight.

Cheers,
Alban