Re: [PATCH] sequencer: trivial cleanup

2013-09-08 Thread Felipe Contreras
On Sun, Sep 8, 2013 at 5:42 PM, Ramkumar Ramachandra artag...@gmail.com wrote:
 Signed-off-by: Ramkumar Ramachandra artag...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] sequencer: trivial cleanup

2013-09-08 Thread Ramkumar Ramachandra
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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] sequencer: trivial cleanup

2013-09-08 Thread SZEDER Gábor
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 artag...@gmail.com 
 wrote:
  Signed-off-by: Ramkumar Ramachandra artag...@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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html