Re: [PATCH v2 2/6] ASoC: ux500: Do not clear state if already idle

From: Mark Brown
Date: Wed May 08 2013 - 09:48:59 EST


On Wed, May 08, 2013 at 02:05:54PM +0100, Lee Jones wrote:

> I'm saying, from experience, from the developer side, that if a
> reviewer goes though a patch-set taking the ones s/he likes leaving
> the rest behind, there are bound to be merge conflicts and semantic
> issues which the developer will then have to resolve. Stuff that I
> believe is added, unnecessary burden which would be easily avoided if
> the set is firstly reviewed and _then_ applied after the Acks have
> been awarded.

So, you have to assume a bit of taste on the part of the people applying
the patches here. If you're seeing merge conflicts that's probably an
issue, but then it's very surprising that the patches would apply
successfully in the first place since the conflicts generally come up
when the patches are applied too.

The other thing to bear in mind here is that a patch series which is
"here's a bunch of random changes to this driver" isn't the same as a
patch series which builds something up through a series of changes.
This series is a good example of the former - there's a few related
things but really there's no visible relationship between most of the
changes except that they happen to be for the same driver and sent at
the same time.

The big downsides of not applying patches are that it takes longer to
get the benefit of the bits that are good out to people and the big
increase in reviewer fatigue from having to re-read the same patches
over and over again. This is one of the major ways problematic code
gets in, reviewers eyes glaze over and they just start missing things.
There's also the fact that serieses often end up having separate bugfix
and development components which need to be routed differently anyway.

Attachment: signature.asc
Description: Digital signature