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
next prev parent 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.