Re: [PATCH 5/7] rebase -i: return control to the caller, for housekeeping
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:56:58
Ramkumar Ramachandra [off-list ref] writes:
On a successful interactive rebase, git-rebase--interactive.sh
currently cleans up and exits on its own. Instead of doing these
two things ourselves:
rm -fr "$dotest"
git gc --auto
let us return control to the caller (git-rebase.sh), to do the
needful. The advantage of doing this is that the caller can implement
a generic cleanup routine (and possibly other things) independent of
specific rebases.
Signed-off-by: Ramkumar Ramachandra <redacted>
---And this answers the question in my review for [4/7]. It would make sense to have these two patch subseries asn three patches (prepare git-rebase.sh, and then convert each backends separately), or a single patch; two patches like this does not make much sense to me.
quoted hunk
git-rebase--interactive.sh | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index cc3a9a7..9514e31 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh@@ -597,7 +597,7 @@ do_next () { fi ;; esac - test -s "$todo" && return + test -s "$todo" && return 1 comment_for_reflog finish && newhead=$(git rev-parse HEAD) &&@@ -623,17 +623,15 @@ do_next () { "$GIT_DIR"/hooks/post-rewrite rebase < "$rewritten_list" true # we don't care if this hook failed fi && - rm -rf "$state_dir" && - git gc --auto && warn "Successfully rebased and updated $head_name." - exit + return 0 } do_rest () { while : do - do_next + do_next && break done }
This is somewhat suspicious. We used to die when do_next failed, or let do_next exit with success. But now you let do_rest return (what does it return???)...
quoted hunk
@@ -799,12 +797,12 @@ first and then run 'git rebase --continue' again." record_in_rewritten "$(cat "$state_dir"/stopped-sha)" require_clean_work_tree "rebase" - do_rest + do_rest && return 0
... and its caller reports success here only when it succeeds. What happens do_rest returns a failure?
;; skip) git rerere clear - do_rest + do_rest && return 0 ;; edit-todo) git stripspace --strip-comments <"$todo" >"$todo".new