Re: [PATCH] Make stashing nothing exit 1

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mar 24, 2019, 03:37 UTC
Message-ID: <CAPig+cT99t1y8kMdGLF_GU0cvaVm=GKMRJ+xRcgS80rRgdQZDQ@mail.gmail.com>
In-Reply-To: <87d0mic9fm.fsf@evledraar.gmail.com>

On Sat, Mar 23, 2019 at 3:54 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 18 quoted lines

> 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.