Re: [Qemu-devel] [PATCH 0/2] m68k: 680x0 processors family support

[Peter Maydell]

On 22 June 2015 at 09:55, Laurent Vivier <address@hidden> wrote:
> Le 22/06/2015 10:33, Peter Maydell a écrit :
>> Thanks for sending this. My initial feeling is that this would
>> really benefit from being split up into more patches to make
>> it easier to review.
>
> Thank you Peter, I understand.
>
> In fact, in my tree, I have 160 commits I have merged in two big ones
> ;-) (some are new features, other bug fixes).
>
> So, no problem, just define the granularity, I will create the patches
> accordingly.
>
> Is it possible to integrate them little by little ?
> I really want to see my patches stack to decrease.

(this is summarising/repeating my remarks on IRC)

Yes, and in fact little by little is my recommendation.
My rule of thumb is that individual patches should be ideally less
than 250 lines in the diffstat. (But they should also be coherent
lumps of functionality, so prefer an occasional larger patch
rather than a completely artificial patch split. It's pretty
rare that something really doesn't have a sensible split point,
though, unless it's a purely mechanical change like a renaming.)

If a patch series gets over 15 or so patches it's pretty oppressive to
review all at once, though. I find the easiest thing to do for this
kind of thing is extract a coherent thematic subset of the total lump
of code, split that into individual patches, and send that as a series.
Once that's reviewed and on its way into the tree, then repeat. This is
how we did the ARM 64-bit support. It means that if there are general
issues (code needing updates to new APIs or best practices, for
example) then they'll be caught in review of the first series, and
you can then get subsequent series right from the start.

As a random example pulled from the archives, this was about
the sixth patchseries for A64 Neon:
https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg01615.html
Each patch implements one insn or a few related insns.

thanks
-- PMM