All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sam Vilain <sam@vilain.net>
To: David Howells <dhowells@redhat.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [Fwd: [PATCH 3/5] NFS: Abstract out namespace initialisation [try #2]]
Date: Fri, 03 Mar 2006 10:53:02 +1300	[thread overview]
Message-ID: <4407693E.6000108@vilain.net> (raw)
In-Reply-To: <5923.1141333943@warthog.cambridge.redhat.com>

David Howells wrote:

>>AIUI, each patch must stand on its own in every regard.  I guess you 
>>need to make it inline in the later patch - or not at all given the 
>>marginal speed difference vs. core size increase.
>>    
>>
>
>No. It has to be permissable to make a series of patches that depend one upon
>another for at least three reasons:
>
> (1) Patches can be unmanageably large in one lump, so splitting them up is a
>     sensible option, even through the individual patches won't work or even
>     compile independently.
>
> (2) It may make sense to place linked changes to two logically separate units
>     in two separate patches, for instance I'm changing the core kernel to add
>     an extra argument to get_sb() and the get_sb_*() convenience functions in
>     one patch and then supplying another patch to change all the filesystems.
>
>     This makes it much easier for a reviewer to see what's going on. They know
>     the patches are interdependent, but they can see the main core of the
>     changes separated out from the massively repetative but basically less
>     interesting changes that are a side effect of the main change.
>
> (3) A series of patches may form a set of logical steps (for instance my
>     patches 1-2 are the first step and patches 3-5 the second). It may be (and
>     it is in my case) that each step will build and run, provided all the
>     previous steps are applied; but that a step won't build or run without the
>     preceding steps.
>
>Remember: one of the main reasons for splitting patches is to make it easier
>for other people to appreciate just how sublimely terrific your work is:-)
>  
>

Interesting.  I've just seen patches slammed by subsystem maintainers 
before for doing things "the wrong way around" within a patchset.

I don't remember seeing this covered in TPP, am I missing having read a 
guide document or is this grey area?

Sam.


  reply	other threads:[~2006-03-02 21:53 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-03-01 21:37 [Fwd: [PATCH 3/5] NFS: Abstract out namespace initialisation [try #2]] Sam Vilain
2006-03-02  8:44 ` Christoph Hellwig
2006-03-02 11:35   ` David Howells
2006-03-02 19:52     ` Sam Vilain
2006-03-02 21:12       ` David Howells
2006-03-02 21:53         ` Sam Vilain [this message]
2006-03-05  0:34           ` Andrew Morton
2006-03-03 16:52         ` J. Bruce Fields
2006-03-02 20:00 ` Sam Vilain

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=4407693E.6000108@vilain.net \
    --to=sam@vilain.net \
    --cc=dhowells@redhat.com \
    --cc=linux-kernel@vger.kernel.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 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.