git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] sequencer: trivial cleanup
@ 2013-09-08 22:42 Ramkumar Ramachandra
  2013-09-08 22:53 ` Felipe Contreras
  0 siblings, 1 reply; 4+ messages in thread
From: Ramkumar Ramachandra @ 2013-09-08 22:42 UTC (permalink / raw)
  To: Git List

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;
 }
 
 static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
-- 
1.8.4.100.gde18f6d.dirty

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] sequencer: trivial cleanup
  2013-09-08 22:42 [PATCH] sequencer: trivial cleanup Ramkumar Ramachandra
@ 2013-09-08 22:53 ` Felipe Contreras
  2013-09-08 22:57   ` Ramkumar Ramachandra
  2013-09-09  0:20   ` SZEDER Gábor
  0 siblings, 2 replies; 4+ messages in thread
From: Felipe Contreras @ 2013-09-08 22:53 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Git List

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.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] sequencer: trivial cleanup
  2013-09-08 22:53 ` Felipe Contreras
@ 2013-09-08 22:57   ` Ramkumar Ramachandra
  2013-09-09  0:20   ` SZEDER Gábor
  1 sibling, 0 replies; 4+ messages in thread
From: Ramkumar Ramachandra @ 2013-09-08 22:57 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Git List

Felipe Contreras wrote:
>>  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.

Yeah, thanks for pointing out this grave stupidity. This seems to be
inconsequential as far as the tests are concerned: have to do some
major yak shaving.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] sequencer: trivial cleanup
  2013-09-08 22:53 ` Felipe Contreras
  2013-09-08 22:57   ` Ramkumar Ramachandra
@ 2013-09-09  0:20   ` SZEDER Gábor
  1 sibling, 0 replies; 4+ messages in thread
From: SZEDER Gábor @ 2013-09-09  0:20 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Ramkumar Ramachandra, Git List

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-09-09  0:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-08 22:42 [PATCH] sequencer: trivial cleanup Ramkumar Ramachandra
2013-09-08 22:53 ` Felipe Contreras
2013-09-08 22:57   ` Ramkumar Ramachandra
2013-09-09  0:20   ` SZEDER Gábor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).