Re: [PATCH 2/2] cherry-pick: fix --quit not deleting CHERRY_PICK_HEAD

On Wed, Aug 15, 2018 at 7:09 PM Stefan Beller [off-list ref] wrote:

Technically you would not need patch 1 in this series, as you could call
remove_branch_state(void) as before that patch to do the same thing here.
I guess that patch 1 is more of a drive by cleanup, then?

Yes.

It looks a bit interesting as sequencer_remove_state actually
takes no arguments and assumes the_repository, but I guess that is fine.

Don't worry. My effort to kill the_index will make sequencer.c take
'struct repository *' (its operations are so wide that passing just
struct index_state * does not make sense).

I wondered if we need to have this patch for 'a' as well, and it looks like
which does a sequencer_rollback, which is just some logic before
attempting sequencer_remove_state. So I'd think remove_branch_state
could be done in sequencer_remove_state as well?

sequencer_rollback does not need this remove_branch_state() line
because it calls reset_for_rollback() which does this deletion. Patch
1/1 kinda hints at that because it touches all remove_branch_state()
;-)

Another part of the reason I did not put remove_branch_state() in
sequencer_remove_state() is I was not sure if it could be used in a
different way (I did not study all of its call sites and am not very
familiar with sequencer code).

Or are there functions that would definitely not want sequencer_remove_state
after sequencer_remove_state?

Going down on that I just realize sequencer_remove_state could also
be returning void, as of now it always returns 0, so the condition here
with !ret is just confusing the reader?

-- 
Duy