Duy Nguyen <pclo...@gmail.com> writes:

>> The internal implementation detail of am_abort() is leaking out
>> here, by saying "rerere-clear" is the only special thing other than
>> recovering the HEAD and working tree state when abort happens.  It
>> makes readers wonder if am_rerere_clear() should become part of
>> am_destroy().  I dunno.
>
> I think the original design is am_destroy takes care of
> $GIT_DIR/rebase-apply and nothing else. --abort has to clean up things
> outside (index, HEAD, rerere) while a successful operation should not
> leave anything else to clean up (except rebase-apply dir).

Yes, and that is why I think the code would have been nicer to
understand if the update to add 'reset' had turned the existing
am_abort() into a helper that is one level higher in the abstraction
(perhaps even by renaming the function) that the caller can tell
which part to clear (i.e. e.g. am_finish(&state, AM_CLEAR_ALL) vs
am_finish(&state, AM_CLEAR_STEP_ONLY)).  Stepping back even further,
perhaps the call made to am_destroy() in the normal exit case at the
end of am_run() could have been using the same helper.



Reply via email to