All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: kusmabite@gmail.com
Cc: Ralf Thielow <ralf.thielow@googlemail.com>,
	git@vger.kernel.org, Valeo de Vries <valeo@valeo.co.cc>
Subject: Re: remove duplicate code and not needed break statement
Date: Mon, 19 Jul 2010 11:06:32 -0500	[thread overview]
Message-ID: <20100719160632.GA17526@burratino> (raw)
In-Reply-To: <AANLkTimHHJnvgFh3Kd7jMqTJJFensZjkD7YCU1gdt-FT@mail.gmail.com>

Hi Erik,

Erik Faye-Lund wrote:
> On Sun, Jul 18, 2010 at 7:49 PM, Ralf Thielow

>>                case 'D': /* we used to emit D but that was misguided. */
>> -                       goto out_stale;
>> -                       break;
>>                case 'T': /* we used to emit T but nobody uses it. */
>
> We have a tendency of adding a comment pointing out fall through
> between case-statements. Perhaps you should add one?

I think in this case that would not be appropriate.  It is the
difference between:

	case 1:
	case 2:
	case 3:
		puts("one to three");
		break;

and

	case 1:
		puts("one");
		/* fall through */
	case 2:
	case 3:
		puts("one to three");
		break;

I admit, I am surprised to see multiple suggestions for improvements
to details of the patch.  I guess I should say, except for the log
message, it is

  Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

i.e., I don’t see any low-hanging fruit for improving it without
actually taking a deeper look at the surrounding code.  Ralf, can we
have your sign-off?  That way maybe it could be applied and people
could suggest patches on top.

Still, thanks for looking it over.

Jonathan

  reply	other threads:[~2010-07-19 16:07 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-18 17:49 remove duplicate code and not needed break statement Ralf Thielow
2010-07-18 17:57 ` Valeo de Vries
     [not found]   ` <AANLkTimuIBe2mvCH15D0RCzlgKTxhHEoX8KEsqiTJgO0@mail.gmail.com>
2010-07-18 18:19     ` Ralf Thielow
2010-07-18 18:29   ` Ralf Thielow
2010-07-18 18:47 ` Jonathan Nieder
2010-07-18 18:56   ` Ralf Thielow
2010-07-19 15:43 ` Erik Faye-Lund
2010-07-19 16:06   ` Jonathan Nieder [this message]
2010-07-19 16:26     ` Ralf Thielow

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100719160632.GA17526@burratino \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=kusmabite@gmail.com \
    --cc=ralf.thielow@googlemail.com \
    --cc=valeo@valeo.co.cc \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.