Junio C Hamano <gitster@xxxxxxxxx> writes:
> Ramkumar Ramachandra <artagnon@xxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html