From: SZEDER Gábor <szeder@ira.uka.de>
Date: Mon, 09 Sep 2013 00:20:27 GMT
Subject: Re: [PATCH] sequencer: trivial cleanup
Message-ID: <20130909002026.GF28959@goldbirke>
In-Reply-To: <CAMP44s1fE0XZScL_gU5JAEdC0hPFHPTovqXLx7gqvwVdZbBaVw@mail.gmail.com>

Hi,

On Sun, Sep 08, 2013 at 05:53:19PM -0500, Felipe Contreras wrote:
> On Sun, Sep 8, 2013 at 5:42 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:
> > Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> > ---
> >  sequencer.c | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/sequencer.c b/sequencer.c
> > index 351548f..8ed9f98 100644
> > --- a/sequencer.c
> > +++ b/sequencer.c
> > @@ -466,10 +466,7 @@ static int allow_empty(struct replay_opts *opts, struct commit *commit)
> >         empty_commit = is_original_commit_empty(commit);
> >         if (empty_commit < 0)
> >                 return empty_commit;
> > -       if (!empty_commit)
> > -               return 0;
> > -       else
> > -               return 1;
> > +       return empty_commit ? 0 : 1;
> >  }
> 
> Isn't it the other way around? Moreover, 'return !!empty_commit;'
> would be simpler.

Considering the possible return values from
is_original_commit_empty(), I think all this could be written as

  return is_original_commit_empty(commit);


Best,
Gábor