Junio C Hamano <gits...@pobox.com> writes:

> Ramkumar Ramachandra <artag...@gmail.com> writes:
>> +apply_autostash () {
>> +    if test -f "$state_dir/autostash"
>> +    then
>> +            stash_sha1=$(cat "$state_dir/autostash")
>> +            git stash apply $stash_sha1 2>&1 >/dev/null ||
>> +            die "
>> +$(eval_gettext 'Applying autostash resulted in conflicts.
>> +Either fix the conflicts now, or run
>> +    git reset --hard
>> +and apply the stash on your desired branch:
>> +    git stash apply $stash_sha1
>> +at any time.')" &&
>> +            echo "Applied autostash"
> That && looks funny.  What does it even mean for die to succeed and
> give control back to you for a chance to say "Applied"?
>       stash apply || die
>       echo applied
> would be far more logical.
>> +    fi
>> +    git gc --auto &&
>> +    rm -rf "$state_dir"
>> +}
> This is somewhat worrisome.

One more thing on this function.  [4/7] (and [5/7]) justified nicely
why it is a good idea to have a central place to do the clean-up
tasks.  apply_autostash is a poor name for that "clean-up" function.
The central clean-up may happen to involve applying a stash in this
version, but applying stash will not stay the entirety of the
clean-up work forever.  The entirety of the clean-up work used to be
just 'git gc --auto && rm -fr "$state_dir"' for eternity, and this
series is adding something else. It is not hard to imagine somebody
else would want to add other kinds of clean-up tasks in here.

Perhaps "finish_rebase" or something?

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to