On Sat, Mar 23, 2019 at 3:54 AM Ævar Arnfjörð Bjarmason
<avarab@xxxxxxxxx> wrote:
> On Sat, Mar 23 2019, Keith Smiley wrote:
> > In the case there are no files to stash, but the user asked to stash, we
> > should exit 1 since the stashing failed.
> > ---
> > diff --git a/git-stash.sh b/git-stash.sh
> > @@ -318,7 +318,7 @@ push_stash () {
> > if no_changes "$@"
> > then
> > say "$(gettext "No local changes to save")"
> > - exit 0
> > + exit 1
> > fi
>
> * Shouldn't we do this consistently across all the other sub-commands?
> Trying some of them seems 'push' may be the odd one out, but maybe
> I've missed some (and this would/should be covered by
> tests). I.e. some single test that does a bunch of ops with no
> entries / nothing to stash and asserts exit codes.
A bigger question is why is this change desirable? What is the
justification for turning this into an error and possibly breaking
existing automation scripts? Arguing that this case should be an
"error" is difficult considering that there are many other commands
(inside and outside of Git) which exit with 0 when they have nothing
to do. I can't find the message in the archive right now, but I recall
a few months ago Junio shooting down an analogous change to some other
command, so the justification needs to be a strong one.
Also, your Signed-off-by: is missing. See
Documentation/SubmittingPatches. Thanks.
