From: Jonathan Corbet <corbet@lwn.net>
To: linux-kernel@vger.kernel.org
Cc: Randy Dunlap <rdunlap@infradead.org>, Jonathan Corbet <corbet@lwn.net>
Subject: [PATCH 1/7] Docs: Remove "tips and tricks" from SubmittingPatches
Date: Tue, 23 Dec 2014 09:32:01 -0700 [thread overview]
Message-ID: <1419352327-4517-2-git-send-email-corbet@lwn.net> (raw)
In-Reply-To: <1419352327-4517-1-git-send-email-corbet@lwn.net>
This section was just a weird collection of stuff that is better found
elsewhere. The "coding style" section somewhat duplicated the previous
coding style section; the useful information there has been collected into
a single place.
Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
Documentation/SubmittingPatches | 117 ++++++++--------------------------------
1 file changed, 21 insertions(+), 96 deletions(-)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 1fa1caa198eb..8f416a2b409f 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -193,17 +193,33 @@ then only post say 15 or so at a time and wait for review and integration.
-4) Style check your changes.
+4) Style-check your changes.
+----------------------------
Check your patch for basic style violations, details of which can be
found in Documentation/CodingStyle. Failure to do so simply wastes
the reviewers time and will get your patch rejected, probably
without even being read.
-At a minimum you should check your patches with the patch style
-checker prior to submission (scripts/checkpatch.pl). You should
-be able to justify all violations that remain in your patch.
+One significant exception is when moving code from one file to
+another -- in this case you should not modify the moved code at all in
+the same patch which moves it. This clearly delineates the act of
+moving the code and your changes. This greatly aids review of the
+actual differences and allows tools to better track the history of
+the code itself.
+
+Check your patches with the patch style checker prior to submission
+(scripts/checkpatch.pl). Note, though, that the style checker should be
+viewed as a guide, not as a replacement for human judgment. If your code
+looks better with a violation then its probably best left alone.
+The checker reports at three levels:
+ - ERROR: things that are very likely to be wrong
+ - WARNING: things requiring careful review
+ - CHECK: things requiring thought
+
+You should be able to justify all violations that remain in your
+patch.
5) Select e-mail destination.
@@ -684,100 +700,9 @@ new/deleted or renamed files.
With rename detection, the statistics are rather different [...]
because git will notice that a fair number of the changes are renames.
------------------------------------
-SECTION 2 - HINTS, TIPS, AND TRICKS
------------------------------------
-
-This section lists many of the common "rules" associated with code
-submitted to the kernel. There are always exceptions... but you must
-have a really good reason for doing so. You could probably call this
-section Linus Computer Science 101.
-
-
-
-1) Read Documentation/CodingStyle
-
-Nuff said. If your code deviates too much from this, it is likely
-to be rejected without further review, and without comment.
-
-One significant exception is when moving code from one file to
-another -- in this case you should not modify the moved code at all in
-the same patch which moves it. This clearly delineates the act of
-moving the code and your changes. This greatly aids review of the
-actual differences and allows tools to better track the history of
-the code itself.
-
-Check your patches with the patch style checker prior to submission
-(scripts/checkpatch.pl). The style checker should be viewed as
-a guide not as the final word. If your code looks better with
-a violation then its probably best left alone.
-
-The checker reports at three levels:
- - ERROR: things that are very likely to be wrong
- - WARNING: things requiring careful review
- - CHECK: things requiring thought
-
-You should be able to justify all violations that remain in your
-patch.
-
-
-
-2) #ifdefs are ugly
-
-Code cluttered with ifdefs is difficult to read and maintain. Don't do
-it. Instead, put your ifdefs in a header, and conditionally define
-'static inline' functions, or macros, which are used in the code.
-Let the compiler optimize away the "no-op" case.
-
-Simple example, of poor code:
-
- dev = alloc_etherdev (sizeof(struct funky_private));
- if (!dev)
- return -ENODEV;
- #ifdef CONFIG_NET_FUNKINESS
- init_funky_net(dev);
- #endif
-
-Cleaned-up example:
-
-(in header)
- #ifndef CONFIG_NET_FUNKINESS
- static inline void init_funky_net (struct net_device *d) {}
- #endif
-
-(in the code itself)
- dev = alloc_etherdev (sizeof(struct funky_private));
- if (!dev)
- return -ENODEV;
- init_funky_net(dev);
-
-
-
-3) 'static inline' is better than a macro
-
-Static inline functions are greatly preferred over macros.
-They provide type safety, have no length limitations, no formatting
-limitations, and under gcc they are as cheap as macros.
-
-Macros should only be used for cases where a static inline is clearly
-suboptimal [there are a few, isolated cases of this in fast paths],
-or where it is impossible to use a static inline function [such as
-string-izing].
-
-'static inline' is preferred over 'static __inline__', 'extern inline',
-and 'extern __inline__'.
-
-
-
-4) Don't over-design.
-
-Don't try to anticipate nebulous future cases which may or may not
-be useful: "Make it as simple as you can, and no simpler."
-
-
----------------------
-SECTION 3 - REFERENCES
+SECTION 2 - REFERENCES
----------------------
Andrew Morton, "The perfect patch" (tpp).
--
2.1.0
next prev parent reply other threads:[~2014-12-23 16:32 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-23 16:32 [PATCH v2] Docs: Modernize SubmittingPatches Jonathan Corbet
2014-12-23 16:32 ` Jonathan Corbet [this message]
2014-12-23 16:32 ` [PATCH 2/7] Docs: Bring SubmittingPatches more into the git era Jonathan Corbet
2016-03-09 9:45 ` David Woodhouse
2016-03-09 11:44 ` Laszlo Ersek
2016-03-09 14:13 ` Jonathan Corbet
2016-03-09 14:04 ` Jonathan Corbet
2016-03-09 14:27 ` David Woodhouse
2014-12-23 16:32 ` [PATCH 3/7] Docs: Update recipient information in SubmittingPatches Jonathan Corbet
2014-12-23 16:32 ` [PATCH 4/7] Docs: SubmittingPatches: update follow-through instructions Jonathan Corbet
2014-12-23 16:32 ` [PATCH 5/7] Docs: SubmittingPatches: miscellaneous cleanups Jonathan Corbet
2014-12-23 16:32 ` [PATCH 6/7] Docs: Mention device tree binding info Jonathan Corbet
2014-12-23 16:32 ` [PATCH 7/7] Docs: SubmittingPatches: mention using pull requests as a cover letter Jonathan Corbet
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=1419352327-4517-2-git-send-email-corbet@lwn.net \
--to=corbet@lwn.net \
--cc=linux-kernel@vger.kernel.org \
--cc=rdunlap@infradead.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®