Re: [Fwd: [PATCH 3/5] NFS: Abstract out namespace initialisation[try #2]]

From: Sam Vilain
Date: Thu Mar 02 2006 - 16:50:50 EST


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.

-
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/