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
