Re: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2 | |
| [Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] | |
On Tue, 2 Sep 2008, Hans Verkuil wrote:
> On Tuesday 02 September 2008 21:37:27 Mike Isely wrote:
> > On Tue, 2 Sep 2008, Mauro Carvalho Chehab wrote:
[...]
> From the CodingStyle document in Linus' git repo:
>
>
> Don't put multiple statements on a single line unless you have
> something to hide:
>
> if (condition) do_this;
> do_something_everytime;
>
> ...
>
> Do not unnecessarily use braces where a single statement will do.
>
> if (condition)
> action();
>
> This does not apply if one branch of a conditional statement is a single
> statement. Use braces in both branches.
>
> if (condition) {
> do_this();
> do_that();
> } else {
> otherwise();
> }
>
>
> So the CodingStyle is correctly enforced by checkpatch.pl.
What? Look at this:
> Don't put multiple statements on a single line unless you have
> something to hide:
>
> if (condition) do_this;
> do_something_everytime;
>
That's the case of:
if (a) b;
And yes, in that case you "do" have something to hide: "b", because it
is conditionally executed based on "a". That's the point of that rule.
But run that by checkpatch.pl and it complains.
I could also point out that
> This does not apply if one branch of a conditional statement is a single
> statement. Use braces in both branches.
does not actually rquire the conditional to have both branches - which
would make
if (a) {
b;
}
legal. But my real bone to pick here is "if (a) b;", which IS allowed
by CodingStyle but not by checkpatch.pl.
> > But you are inflicting it on others here. So expect to hear something
> > about it, at least from me. I'm not going to yell at the gun
> > manufacturer if you're the one pointing the gun at me.
>
> It's Mauro's job as subsystem maintainer to enforce such rules. It's
> like arguing with a cop about a speeding ticket. Utterly pointless. If
> you want to change the speed limit you argue with the guys who make the
> law, and that ain't the cop.
If you want to use that analogy, then Mauro is pulling over anyone doing
51MPH in a 50MPH zone. In the US at least, that never happens because
if it was attempted then every single driver would be pulled over. Cops
are interested in real, egregious cases, those generally doing more than
10MPH over the limit. Cops here apply subjective judgement to the
situation; Mauro is not. (Now, one might argue that this subjective
judgement in the case of speeding is also a bad thing, and I would agree
with you, but that only gets solved by setting the speed limit to
something realistic, and that's going way off topic here.)
>
> Mike, I'm sorry to say this but I think this is a senseless discussion.
> It's a totally unimportant facet of the driver and there are a zillion
> more important things to worry about. Be glad the kernel doesn't use
> the GNU codingstyle, that's *really* horrible. Each company or
> organization has (usually) a coding style, and this is, for better or
> worse, the Linux coding style. Just close your eyes, remove the curly
> brackets and commit and get on to the next much more important
> bug/problem/feature/whatever.
You know, I agree with you, to a limit. But I have been grinding my
teeth over this for the past year, and it's reaching my limit. The
engineer in me says to fix the busted rule, and I've tried without
success. Now the cop in me says that because the rule is unfixable then
it must be interpreted subjectively - if the script is flawed, admit
that and use that knowledge when deciding what to whack people about.
But Mauro isn't doing that either.
You point out that this is a petty problem. It sure is. I could also
point out that it is a petty thing to argue about in code reviews as
well.
I will do what's been agreed to here, because it still preserves my
notion of what I think is right.
But going forward, I don't know. As I said, this has caused me a great
deal of grief over an extended period of time. I guess this was just
the final straw. Perhaps you think it shouldn't be such an issue.
Well that's your take. I can tell you what it *IS* doing to me, and I
don't like it. I generally like the kernel CodingStyle. It tries to do
the right things in the right places. The checkpatch script has, for me
at least, two critical flaws. (The other is space-after-comma.) I am
finding myself worrying more about checkpatch than actually solving the
'real' problems at hand. That is bad. I am sick of it. I do not know
what I will do. Probably I won't do anything pro-active, but if I
continue to get flack about petty things like this then I will probably
just mentally give up and go find less stressful things to do.
-Mike
--
Mike Isely
isely @ pobox (dot) com
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
_______________________________________________
v4l-dvb-maintainer mailing list
v4l-dvb-maintainer@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/v4l-dvb-maintainer
|
|
-->
