Message-ID: <548F63DA.5040702@mvista.com>
Date:	Mon, 15 Dec 2014 16:42:34 -0600
From:	Corey Minyard <cminyard@...sta.com>
To:	Jonathan Corbet <corbet@....net>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RFC] Docs: Modernize SubmittingPatches

No comments besides the ones Randy already mentioned.

Thanks,

-corey

On 12/15/2014 09:52 AM, Jonathan Corbet wrote:
> The SubmittingPatches file still shows a lot of its roots from the era when
> we all sent stuff straight to Linus and hoped for the best.  I've gone in
> and thrashed it up to reflect an age where few of us type our own "diff"
> commands anymore.  Also added a section on preparing signed tags for pull
> requests.
>
> Signed-off-by: Jonathan Corbet <corbet@....net>
> ---
>  Documentation/SubmittingPatches | 433 ++++++++++++++++++++--------------------
>  1 file changed, 212 insertions(+), 221 deletions(-)
>
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index 1fa1caa198eb..787d0711e18a 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -10,27 +10,48 @@ kernel, the process can sometimes be daunting if you're not familiar
>  with "the system."  This text is a collection of suggestions which
>  can greatly increase the chances of your change being accepted.
>  
> -Read Documentation/SubmitChecklist for a list of items to check
> -before submitting code.  If you are submitting a driver, also read
> +This document contains a large number of suggestions in a relatively terse
> +format.  For detailed information on how the kernel development process
> +works, see Documentation/development-process.  Also, read
> +Documentation/SubmitChecklist for a list of items to check before
> +submitting code.  If you are submitting a driver, also read
>  Documentation/SubmittingDrivers.
>  
>  Many of these steps describe the default behavior of the git version
>  control system; if you use git to prepare your patches, you'll find much
>  of the mechanical work done for you, though you'll still need to prepare
> -and document a sensible set of patches.
> +and document a sensible set of patches.  In general, use of git will make
> +your life as a kernel developer easier.
>  
>  --------------------------------------------
>  SECTION 1 - CREATING AND SENDING YOUR CHANGE
>  --------------------------------------------
>  
>  
> +0) Obtain a current source tree
> +-------------------------------
> +
> +If you do not have a repository with the current kernel source handy, use
> +git to obtain one.  You'll want to start with the mainline repository,
> +which can be grabbed with:
> +
> +  git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
> +
> +Note, however, that you may not want to develop against the mainline tree
> +directly.  Most subsystem maintainers run their own trees and want to see
> +patches prepared against those trees.  See the "T:" entry for the subsystem
> +in the MAINTAINERS file to find that tree, or simply ask the maintainer if
> +the tree is not listed there.
> +
> +It is still possible to download kernel releases via tarballs (as described
> +in the next section), but that is the hard way to do kernel development.
>  
>  1) "diff -up"
>  ------------
>  
> -Use "diff -up" or "diff -uprN" to create patches.  git generates patches
> -in this form by default; if you're using git, you can skip this section
> -entirely.
> +If you must generate your patches by hand, use "diff -up" or "diff -uprN"
> +to create patches.  Git generates patches in this form by default; if
> +you're using git, you can skip this section entirely.
>  
>  All changes to the Linux kernel occur in the form of patches, as
>  generated by diff(1).  When creating your patch, make sure to create it
> @@ -42,7 +63,7 @@ not in any lower subdirectory.
>  
>  To create a patch for a single file, it is often sufficient to do:
>  
> -	SRCTREE= linux-2.6
> +	SRCTREE= linux
>  	MYFILE=  drivers/net/mydriver.c
>  
>  	cd $SRCTREE
> @@ -55,17 +76,16 @@ To create a patch for multiple files, you should unpack a "vanilla",
>  or unmodified kernel source tree, and generate a diff against your
>  own source tree.  For example:
>  
> -	MYSRC= /devel/linux-2.6
> +	MYSRC= /devel/linux
>  
> -	tar xvfz linux-2.6.12.tar.gz
> -	mv linux-2.6.12 linux-2.6.12-vanilla
> -	diff -uprN -X linux-2.6.12-vanilla/Documentation/dontdiff \
> -		linux-2.6.12-vanilla $MYSRC > /tmp/patch
> +	tar xvfz linux-3.19.tar.gz
> +	mv linux-3.19 linux-3.19-vanilla
> +	diff -uprN -X linux-3.19-vanilla/Documentation/dontdiff \
> +		linux-3.19-vanilla $MYSRC > /tmp/patch
>  
>  "dontdiff" is a list of files which are generated by the kernel during
>  the build process, and should be ignored in any diff(1)-generated
> -patch.  The "dontdiff" file is included in the kernel tree in
> -2.6.12 and later.
> +patch.
>  
>  Make sure your patch does not include any extra files which do not
>  belong in a patch submission.  Make sure to review your patch -after-
> @@ -83,6 +103,7 @@ is another popular alternative.
>  
>  
>  2) Describe your changes.
> +-------------------------
>  
>  Describe your problem.  Whether your patch is a one-line bug fix or
>  5000 lines of a new feature, there must be an underlying problem that
> @@ -124,10 +145,10 @@ See #3, next.
>  When you submit or resubmit a patch or patch series, include the
>  complete patch description and justification for it.  Don't just
>  say that this is version N of the patch (series).  Don't expect the
> -patch merger to refer back to earlier patch versions or referenced
> +subsystem maintainer to refer back to earlier patch versions or referenced
>  URLs to find the patch description and put that into the patch.
>  I.e., the patch (series) and its description should be self-contained.
> -This benefits both the patch merger(s) and reviewers.  Some reviewers
> +This benefits both the maintainers and reviewers.  Some reviewers
>  probably didn't even receive earlier versions of the patch.
>  
>  Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
> @@ -156,10 +177,15 @@ Example:
>  	platform_set_drvdata(), but left the variable "dev" unused,
>  	delete it.
>  
> +You should also be sure to use at least the first twelve characters of the
> +SHA-1 ID.  The kernel repository holds a *lot* of objects, making
> +collisions with shorter IDs a real possibility.  Bear in mind that, even if
> +there is no collision with your six-character ID now, that condition may
> +change five years from now.
> +
>  If your patch fixes a bug in a specific commit, e.g. you found an issue using
>  git-bisect, please use the 'Fixes:' tag with the first 12 characters of the
> -SHA-1 ID, and the one line summary.
> -Example:
> +SHA-1 ID, and the one line summary.  For example:
>  
>  	Fixes: e21d2170f366 ("video: remove unnecessary platform_set_drvdata()")
>  
> @@ -172,8 +198,9 @@ outputting the above style in the git log or git show commands
>  		fixes = Fixes: %h (\"%s\")
>  
>  3) Separate your changes.
> +-------------------------
>  
> -Separate _logical changes_ into a single patch file.
> +Separate each _logical change_ into a separate patch.
>  
>  For example, if your changes include both bug fixes and performance
>  enhancements for a single driver, separate those changes into two
> @@ -184,90 +211,114 @@ On the other hand, if you make a single change to numerous files,
>  group those changes into a single patch.  Thus a single logical change
>  is contained within a single patch.
>  
> +The point to remember is that each patch should make an easily understood
> +change that can be verified by reviewers.  Each patch should be justifiable
> +on its own merits.
> +
>  If one patch depends on another patch in order for a change to be
>  complete, that is OK.  Simply note "this patch depends on patch X"
>  in your patch description.
>  
> +When dividing your change into a series of patches, take special care to
> +ensure that the kernel builds and runs properly after each patch in the
> +series.  Developers using "git bisect" to track down a problem can end up
> +splitting your patch series at any point; they will not thank you if you
> +introduce bugs in the middle.
> +
>  If you cannot condense your patch set into a smaller set of patches,
>  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
>  
> -5) Select e-mail destination.
> +You should be able to justify all violations that remain in your
> +patch.
>  
> -Look through the MAINTAINERS file and the source code, and determine
> -if your change applies to a specific subsystem of the kernel, with
> -an assigned maintainer.  If so, e-mail that person.  The script
> -scripts/get_maintainer.pl can be very useful at this step.
>  
> -If no maintainer is listed, or the maintainer does not respond, send
> -your patch to the primary Linux kernel developer's mailing list,
> -linux-kernel@...r.kernel.org.  Most kernel developers monitor this
> -e-mail list, and can comment on your changes.
> +5) Select the recipients for your patch.
> +----------------------------------------
>  
> +You should always copy the appropriate subsystem maintainer(s) on any patch
> +to code that they maintain; Look through the MAINTAINERS file and the
> +source code revision history to see who those maintainers are.  The
> +script scripts/get_maintainer.pl can be very useful at this step.  If you
> +cannot find a maintainer for the subsystem your are working on, Andrew
> +Morton (akpm@...ux-foundation.org) serves as a maintainer of last resort.
>  
> -Do not send more than 15 patches at once to the vger mailing lists!!!
> +You should also normally choose at least one mailing list to receive a copy
> +of your patch set.  linux-kernel@...r.kernel.org functions as a list of
> +last resort, but the volume on that list has caused a number of developers
> +to tune it out.  Look in the MAINTAINERS file for a subsystem-specific
> +list; your patch will probably get more attention there.  Please do not
> +spam unrelated lists, though.
>  
> +Many kernel-related lists are hosted on vger.kernel.org; you can find a
> +list of them at http://vger.kernel.org/vger-lists.html.  There are
> +kernel-related lists hosted elsewhere as well, though.
> +
> +Do not send more than 15 patches at once to the vger mailing lists!!!
>  
>  Linus Torvalds is the final arbiter of all changes accepted into the
>  Linux kernel.  His e-mail address is <torvalds@...ux-foundation.org>. 
> -He gets a lot of e-mail, so typically you should do your best to -avoid-
> -sending him e-mail. 
> +He gets a lot of e-mail, and, at this point, very few patches go through
> +Linus directly, so typically you should do your best to -avoid-
> +sending him e-mail.
>  
> -Patches which are bug fixes, are "obvious" changes, or similarly
> -require little discussion should be sent or CC'd to Linus.  Patches
> -which require discussion or do not have a clear advantage should
> -usually be sent first to linux-kernel.  Only after the patch is
> -discussed should the patch then be submitted to Linus.
> +If you have a patch that fixes an exploitable security bug, send that patch
> +to security@...nel.org.  For severe bugs, a short embargo may be considered
> +to allow distrbutors to get the patch out to users; in such cases,
> +obviously, the patch should not be sent to any public lists.
>  
> +Patches that fix a severe bug in a released kernel should be directed
> +toward the stable maintainers; putting a line like this:
>  
> +  Cc: stable@...r.kernel.org
>  
> -6) Select your CC (e-mail carbon copy) list.
> +Note, however, that some subsystem maintainers want to come to their own
> +conclusions on which patches should go to the stable trees.  The networking
> +maintainer, in particular, would rather not see individual developers
> +adding lines like the above to their patches.
>  
> -Unless you have a reason NOT to do so, CC linux-kernel@...r.kernel.org.
> -
> -Other kernel developers besides Linus need to be aware of your change,
> -so that they may comment on it and offer code review and suggestions.
> -linux-kernel is the primary Linux kernel developer mailing list.
> -Other mailing lists are available for specific subsystems, such as
> -USB, framebuffer devices, the VFS, the SCSI subsystem, etc.  See the
> -MAINTAINERS file for a mailing list that relates specifically to
> -your change.
> -
> -Majordomo lists of VGER.KERNEL.ORG at:
> -	<http://vger.kernel.org/vger-lists.html>
> -
> -If changes affect userland-kernel interfaces, please send
> -the MAN-PAGES maintainer (as listed in the MAINTAINERS file)
> -a man-pages patch, or at least a notification of the change,
> -so that some information makes its way into the manual pages.
> -
> -Even if the maintainer did not respond in step #5, make sure to ALWAYS
> -copy the maintainer when you change their code.
> +If changes affect userland-kernel interfaces, please send the MAN-PAGES
> +maintainer (as listed in the MAINTAINERS file) a man-pages patch, or at
> +least a notification of the change, so that some information makes its way
> +into the manual pages.  User-space API changes should also be copied to
> +linux-api@...r.kernel.org. 
>  
>  For small patches you may want to CC the Trivial Patch Monkey
>  trivial@...nel.org which collects "trivial" patches. Have a look
>  into the MAINTAINERS file for its current manager.
>  Trivial patches must qualify for one of the following rules:
>   Spelling fixes in documentation
> - Spelling fixes which could break grep(1)
> + Spelling fixes for errors which could break grep(1)
>   Warning fixes (cluttering with useless warnings is bad)
>   Compilation fixes (only if they are actually correct)
>   Runtime fixes (only if they actually fix things)
> - Removing use of deprecated functions/macros (eg. check_region)
> + Removing use of deprecated functions/macros
>   Contact detail and documentation fixes
>   Non-portable code replaced by portable code (even in arch-specific,
>   since people copy, as long as it's trivial)
> @@ -276,7 +327,8 @@ Trivial patches must qualify for one of the following rules:
>  
>  
>  
> -7) No MIME, no links, no compression, no attachments.  Just plain text.
> +6) No MIME, no links, no compression, no attachments.  Just plain text.
> +-----------------------------------------------------------------------
>  
>  Linus and other kernel developers need to be able to read and comment
>  on the changes you are submitting.  It is important for a kernel
> @@ -299,54 +351,48 @@ you to re-send them using MIME.
>  See Documentation/email-clients.txt for hints about configuring
>  your e-mail client so that it sends your patches untouched.
>  
> -8) E-mail size.
> -
> -When sending patches to Linus, always follow step #7.
> +7) E-mail size.
> +---------------
>  
>  Large changes are not appropriate for mailing lists, and some
>  maintainers.  If your patch, uncompressed, exceeds 300 kB in size,
>  it is preferred that you store your patch on an Internet-accessible
> -server, and provide instead a URL (link) pointing to your patch.
> +server, and provide instead a URL (link) pointing to your patch.  But note
> +that if your patch exceeds 300kB, it almost certainly needs to be broken up
> +anyway.
>  
> +8) Respond to review comments.
> +------------------------------
>  
> +Your patch will almost certainly get comments from reviewers on ways in
> +which the patch can be improved.  You must respond to those comments;
> +ignoring reviewers is a good way to get ignored in return.  Review comments
> +or questions that to not lead to a code change should almost certainly
> +bring about a comment or changelog entry so that the next reviewer better
> +understands what is going on.
>  
> -9) Name your kernel version.
> +Be sure to tell the reviewers what changes you are making and to thank them
> +for their time.  Code review is a tiring and time-consuming process, and
> +reviewers sometimes get grumpy.  Even in that case, though, respond
> +politely and address the problems they have pointed out.
>  
> -It is important to note, either in the subject line or in the patch
> -description, the kernel version to which this patch applies.
>  
> -If the patch does not apply cleanly to the latest kernel version,
> -Linus will not apply it.
> +9) Don't get discouraged - or impatient.
> +----------------------------------------
>  
> +After you have submitted your change, be patient and wait.  Reviewers are
> +busy people and may not get to your patch right away.
>  
> +Once upon a time, patches used to dissappear into the void without comment,
> +but the development process works more smoothly than that now.  You should
> +receive comments within a week or so; if that does not happen, make sure
> +that you have sent your patches to the right place.  Wait for a minimum of
> +one week before resubmitting or pinging reviewers - possibly longer during
> +busy times like merge windows.
>  
> -10) Don't get discouraged.  Re-submit.
>  
> -After you have submitted your change, be patient and wait.  If Linus
> -likes your change and applies it, it will appear in the next version
> -of the kernel that he releases.
> -
> -However, if your change doesn't appear in the next version of the
> -kernel, there could be any number of reasons.  It's YOUR job to
> -narrow down those reasons, correct what was wrong, and submit your
> -updated change.
> -
> -It is quite common for Linus to "drop" your patch without comment.
> -That's the nature of the system.  If he drops your patch, it could be
> -due to
> -* Your patch did not apply cleanly to the latest kernel version.
> -* Your patch was not sufficiently discussed on linux-kernel.
> -* A style issue (see section 2).
> -* An e-mail formatting issue (re-read this section).
> -* A technical problem with your change.
> -* He gets tons of e-mail, and yours got lost in the shuffle.
> -* You are being annoying.
> -
> -When in doubt, solicit comments on linux-kernel mailing list.
> -
> -
> -
> -11) Include PATCH in the subject
> +10) Include PATCH in the subject
> +--------------------------------
>  
>  Due to high e-mail traffic to Linus, and to linux-kernel, it is common
>  convention to prefix your subject line with [PATCH].  This lets Linus
> @@ -355,7 +401,8 @@ e-mail discussions.
>  
>  
>  
> -12) Sign your work
> +11) Sign your work
> +------------------
>  
>  To improve tracking of who did what, especially with patches that can
>  percolate to their final resting place in the kernel through several
> @@ -429,15 +476,15 @@ which appears in the changelog.
>  Special note to back-porters: It seems to be a common and useful practice
>  to insert an indication of the origin of a patch at the top of the commit
>  message (just after the subject line) to facilitate tracking. For instance,
> -here's what we see in 2.6-stable :
> +here's what we see in a 3.x-stable release:
>  
> -    Date:   Tue May 13 19:10:30 2008 +0000
> +Date:   Tue Oct 7 07:26:38 2014 -0400
>  
> -        SCSI: libiscsi regression in 2.6.25: fix nop timer handling
> +    libata: Un-break ATA blacklist
>  
> -        commit 4cf1043593db6a337f10e006c23c69e5fc93e722 upstream
> +    commit 1c40279960bcd7d52dbdf1d466b20d24b99176c8 upstream.
>  
> -And here's what appears in 2.4 :
> +And here's what might appear in an older kernel once a patch is backported:
>  
>      Date:   Tue May 13 22:12:27 2008 +0200
>  
> @@ -446,18 +493,19 @@ And here's what appears in 2.4 :
>          [backport of 2.6 commit b7acbdfbd1f277c1eb23f344f899cfa4cd0bf36a]
>  
>  Whatever the format, this information provides a valuable help to people
> -tracking your trees, and to people trying to trouble-shoot bugs in your
> +tracking your trees, and to people trying to troubleshoot bugs in your
>  tree.
>  
>  
> -13) When to use Acked-by: and Cc:
> +12) When to use Acked-by: and Cc:
> +---------------------------------
>  
>  The Signed-off-by: tag indicates that the signer was involved in the
>  development of the patch, or that he/she was in the patch's delivery path.
>  
>  If a person was not directly involved in the preparation or handling of a
>  patch but wishes to signify and record their approval of it then they can
> -arrange to have an Acked-by: line added to the patch's changelog.
> +ask to have an Acked-by: line added to the patch's changelog.
>  
>  Acked-by: is often used by the maintainer of the affected code when that
>  maintainer neither contributed to nor forwarded the patch.
> @@ -465,7 +513,8 @@ maintainer neither contributed to nor forwarded the patch.
>  Acked-by: is not as formal as Signed-off-by:.  It is a record that the acker
>  has at least reviewed the patch and has indicated acceptance.  Hence patch
>  mergers will sometimes manually convert an acker's "yep, looks good to me"
> -into an Acked-by:.
> +into an Acked-by: (but note that it is usually better to ask for an
> +explicit ack).
>  
>  Acked-by: does not necessarily indicate acknowledgement of the entire patch.
>  For example, if a patch affects multiple subsystems and has an Acked-by: from
> @@ -477,11 +526,13 @@ list archives.
>  If a person has had the opportunity to comment on a patch, but has not
>  provided such comments, you may optionally add a "Cc:" tag to the patch.
>  This is the only tag which might be added without an explicit action by the
> -person it names.  This tag documents that potentially interested parties
> -have been included in the discussion
> +person it names - but it should indicate that this person was copied on the
> +patch.  This tag documents that potentially interested parties
> +have been included in the discussion.
>  
>  
> -14) Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:
> +13) Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:
> +--------------------------------------------------------------------------
>  
>  The Reported-by tag gives credit to people who find bugs and report them and it
>  hopefully inspires them to help us again in the future.  Please note that if
> @@ -541,7 +592,13 @@ which stable kernel versions should receive your fix. This is the preferred
>  method for indicating a bug fixed by the patch. See #2 above for more details.
>  
>  
> -15) The canonical patch format
> +14) The canonical patch format
> +------------------------------
> +
> +This section describes how the patch itself should be formatted.  Note
> +that, if you have your patches stored in a git repository, proper patch
> +formatting can be had with "git format-patch".  The tools can not create
> +the necessary text, though, so read the instructions below anyway.
>  
>  The canonical patch subject line is:
>  
> @@ -549,7 +606,8 @@ The canonical patch subject line is:
>  
>  The canonical patch message body contains the following:
>  
> -  - A "from" line specifying the patch author.
> +  - A "from" line specifying the patch author (only needed if the person
> +    sending the patch is not the author).
>  
>    - An empty line.
>  
> @@ -656,128 +714,61 @@ See more details on the proper patch format in the following
>  references.
>  
>  
> -16) Sending "git pull" requests  (from Linus emails)
> -
> -Please write the git repo address and branch name alone on the same line
> -so that I can't even by mistake pull from the wrong branch, and so
> -that a triple-click just selects the whole thing.
> -
> -So the proper format is something along the lines of:
> -
> -	"Please pull from
> -
> -		git://jdelvare.pck.nerim.net/jdelvare-2.6 i2c-for-linus
> -
> -	 to get these changes:"
> -
> -so that I don't have to hunt-and-peck for the address and inevitably
> -get it wrong (actually, I've only gotten it wrong a few times, and
> -checking against the diffstat tells me when I get it wrong, but I'm
> -just a lot more comfortable when I don't have to "look for" the right
> -thing to pull, and double-check that I have the right branch-name).
> -
> -
> -Please use "git diff -M --stat --summary" to generate the diffstat:
> -the -M enables rename detection, and the summary enables a summary of
> -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
> +15) Sending "git pull" requests
> +-------------------------------
>  
> -(in the code itself)
> -	dev = alloc_etherdev (sizeof(struct funky_private));
> -	if (!dev)
> -		return -ENODEV;
> -	init_funky_net(dev);
> +If you have a series of patches, it may be most convenient to have the
> +maintainer pull them directly into the subsystem repository with a
> +"git pull" operation.  Note, however, that pulling patches from a developer
> +requires a higher degree of trust than taking patches from a mailing list.
> +As a result, many subsystem maintainers are reluctant to take pull
> +requests, especially from new, unknown developers.
>  
> +A pull request should have [GIT] or [PULL] in the subject line.  The
> +request itself should include the repository name and the branch of
> +interest on a single line; it should look something like:
>  
> +  Please pull from
>  
> -3) 'static inline' is better than a macro
> +      git://jdelvare.pck.nerim.net/jdelvare-2.6 i2c-for-linus
>  
> -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.
> +  to get these changes:"
>  
> -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].
> +A pull request should also include an overall message saying what will be
> +included in the request, a "git shortlog" listing of the patches
> +themselves, and a diffstat showing the overall effect of the patch series.
> +The easiest way to get all this information together is, of course, to let
> +git do it for you with the "git request-pull" command.
>  
> -'static inline' is preferred over 'static __inline__', 'extern inline',
> -and 'extern __inline__'.
> +Some maintainers (including Linus) want to see pull requests from signed
> +commits; that increases their confidence that the request actually came
> +from you.  Linus, in particular, will not pull from public hosting sites
> +like GitHub in the absence of a signed tag.
>  
> +The first step toward creating such tags is to make a GNUPG key and get it
> +signed by one or more core kernel developers.  This step can be hard for
> +new developers, but there is no way around it.  Attending conferences can
> +be a good way to find developers who can sign your key.
>  
> +Once you have prepared a patch series in git that you wish to have somebody
> +pull, create a signed tag with "git tag -s".  This will create a new tag
> +identifying the last commit in the series and containing a signature
> +created with your private key.  You will also have the opportunity to add a
> +changelog-style message to the tag; this is an ideal place to describe the
> +effects of the pull request as a whole.
>  
> -4) Don't over-design.
> +If the tree the maintainer will be pulling from is not the repository you
> +are working from, don't forget to push the signed tag explicitly to the
> +public tree.
>  
> -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."
> +When generating your pull request, use the signed tag as the target.  A
> +command like this will do the trick:
>  
> +  git request-pull master git://my.public.tree/linux.git my-signed-tag
>  
>  
>  ----------------------
> -SECTION 3 - REFERENCES
> +SECTION 2 - REFERENCES
>  ----------------------
>  
>  Andrew Morton, "The perfect patch" (tpp).

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

Powered by blists - more mailing lists