Re: [PATCH] update checkpatch.pl to version 0.03

From: jschopp
Date: Mon Jun 04 2007 - 13:22:27 EST


The original suggestion was to count them and only complain if there
were "lots". I had thought though that the general consensus was that
#ifdef in C files was pretty much frowned upon. I must admit to working
to the "you must be able to justify all winges in the output" rather
than expecting the result to be empty necessarily.

I really think we should work towards, "if you see an error the odds are overwhelming that we aren't wasting your time and you should fix this." rather than, "every time you send a patch you will get a couple false positives that you will have to think about and justify to yourself, slowing down your sending the patch out and making more work for you". I'm not saying we have to have 100% accuracy, but I want it well in the 90s.

Now back to the ifdef's. I don't think we can really even say a lot or a little is broken (short patch can do 1 ifdef that is stupid, long patch could do several that are good). I think we'll either have to let that one go or put it under a non-default flag. We do still have humans reviewing code who can make judgement calls like if you #ifdefs are good or not.

What we can check for is #if 0 code. I hate that one.

We've not talked about how the tool might grow. My thought was that
soon we would enable the robot replies on linux-mm (say) and use the
feedback from that to tune what we do and do not report. I have been
pushing all of the contributions to -mm for sometime through it and
cirtainly the #ifdef one once of the more common ones (other than white
space dammage and >80 character lines).

If you have reasonably good SPAM filters on your list and make the robot so it is very good about only picking up mail that really are patches then this could be a very good idea. Just be careful. I could send out a bunch of mail with fake headers saying it was from say Andy Whitcroft, then the robot replies and you got a bunch of junk mail.

User feedback is really useful, both for us and for the user. Based on user feedback from the very small number of users I had I tweaked a lot of regular expressions, added some new checks, and removed some I could never get right.

-Joel
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/