Ramkumar Ramachandra <artagnon@xxxxxxxxx> 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 <artagnon@xxxxxxxxx>
> ---
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.
> 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???)...
>
> @@ -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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html