public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Lee Jones <lee.jones@linaro.org>
To: Mark Brown <broonie@kernel.org>
Cc: Fabio Baltieri <fabio.baltieri@linaro.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
	Linus Walleij <linus.walleij@linaro.org>,
	Ola Lilja <ola.o.lilja@stericsson.com>
Subject: Re: [PATCH v2 2/6] ASoC: ux500: Do not clear state if already idle
Date: Wed, 8 May 2013 13:03:26 +0100	[thread overview]
Message-ID: <20130508120326.GF3459@gmail.com> (raw)
In-Reply-To: <20130508113124.GH7478@sirena.org.uk>

On Wed, 08 May 2013, Mark Brown wrote:

> On Wed, May 08, 2013 at 12:04:51PM +0100, Lee Jones wrote:
> > On Wed, 08 May 2013, Mark Brown wrote:
> 
> > > Ugh, please don't do stuff like this - you're posting an individual
> > > revision of a patch buried in the middle of a thread.  This just makes
> > > things hard to follow and error prone.  Repost the patch series
> 
> > It's so much more convenient to do it this way. Re-sending entire
> > patch-sets for small fixups is clumsy and annoying at best. Creating
> > even more prone to error.
> 
> Then consider what happens as soon as you get more than one update to
> the patch, or there's any meaningful discussion on the patches - picking
> out which of many versions in bifurcating threads.  You shouldn't even
> assume that followups to the patches are being read, for example if it's
> clear that there's revision required and it's all device specific
> discussion rather than framework stuff I'll often just stop reading the
> thread and wait for the respost.

<snip>

> > Surely most people have their mail setup as threaded? Then the
> > time-line and subsequent patch versions are very easy to follow aren't
> > they? I get a nice trace like this:

<snip>

> This doesn't work nearly so well once you start getting meaningful
> discussion, multiple branches on the thread and indentation can make it
> hard to spot where the latest patch is and it's still more effort to
> find the latest version.

Yes, of course one still has to use common sense. If this happens then
I'd say a re-post would be the obvious thing to do. I'm speaking more
about situations such as this, where the discussion is trivial and the
fixup, less so.

> > > or wait until what can be applied is applied then repost.
> 
> > Taking patches out-of-order, or 'willy-nilly', is asking for trouble.
> 
> We've been through this repeatedly.  If your early patches won't work
> without the later patches then you need to improve your early patches so
> they stand alone.

It's never okay for early patches to rely on later ones and yes, all
patches should be as orthogonal as possible. But it is okay for later
patches to rely on earlier ones.

Besides, I was more referencing the massively increased effort
imparted to the developer by applying patches in an arbitrary order.
Forcing the developer to rearranging and rebase the patch-set causing
unnecessary merge conflicts. It's better if the maintainer takes the
patch-set in the order it was written to prevent unnecessary (which is
the key word here) such things.

> Asking people to re-review an enormous patch series
> because of a change at the end isn't helpful

I completely agree. Hence why I reap Acks and apply them on next
submission to show the maintainer what they have reviewed and accepted
already. 

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

  reply	other threads:[~2013-05-08 12:03 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-08  7:14 [PATCH 0/6] Second set of fixes for ux500 ASoC drivers Fabio Baltieri
2013-05-08  7:14 ` [PATCH 1/6] ASoC: ab8500-codec: Add missing ad_to_slot definitions Fabio Baltieri
2013-05-08  7:53   ` Lee Jones
2013-05-08  8:30     ` Fabio Baltieri
2013-05-08  8:47       ` Lee Jones
2013-05-08 10:58   ` Mark Brown
2013-05-08  7:14 ` [PATCH 2/6] ASoC: ux500: Do not clear state if already idle Fabio Baltieri
2013-05-08  8:04   ` Lee Jones
2013-05-08  8:39     ` [PATCH v2 " Fabio Baltieri
2013-05-08 10:34       ` Mark Brown
2013-05-08 11:04         ` Lee Jones
2013-05-08 11:31           ` Mark Brown
2013-05-08 12:03             ` Lee Jones [this message]
2013-05-08 12:39               ` Mark Brown
2013-05-08 13:05                 ` Lee Jones
2013-05-08 13:48                   ` Mark Brown
2013-05-08 14:06                     ` Lee Jones
2013-05-09  9:28                       ` Mark Brown
2013-05-08 12:04         ` Fabio Baltieri
2013-05-08 12:39           ` Mark Brown
2013-05-08  7:14 ` [PATCH 3/6] ASoC: ux500: Drop pinctrl sleep support Fabio Baltieri
2013-05-08  8:07   ` Lee Jones
2013-05-08  8:20     ` Fabio Baltieri
2013-05-08  8:48       ` Lee Jones
2013-05-08  9:00         ` Fabio Baltieri
2013-05-08 10:51   ` Mark Brown
2013-05-08 11:42     ` Fabio Baltieri
2013-05-08 12:32       ` Mark Brown
2013-05-08 13:10         ` Fabio Baltieri
2013-05-08 13:54           ` Mark Brown
2013-05-08 14:17             ` Fabio Baltieri
2013-05-08 14:27               ` Fabio Baltieri
2013-05-08 14:49                 ` Mark Brown
2013-05-08 15:07                   ` Lee Jones
2013-05-09  9:34                     ` Mark Brown
2013-05-08 14:29               ` Mark Brown
2013-05-08 15:48                 ` Fabio Baltieri
2013-05-09  9:41                   ` Mark Brown
2013-05-13 10:43                     ` Fabio Baltieri
2013-05-17 22:02                   ` Linus Walleij
2013-05-08  7:14 ` [PATCH 4/6] ASoC: ux500: Update tx tdm slots configuration Fabio Baltieri
2013-05-08  8:18   ` Lee Jones
2013-05-08 11:01   ` Mark Brown
2013-05-08 11:11     ` Lee Jones
2013-05-08 11:32       ` Fabio Baltieri
2013-05-08 12:28       ` Mark Brown
2013-05-08 16:03     ` Fabio Baltieri
2013-05-08  7:14 ` [PATCH 5/6] ASoC: ux500: Swap even/odd AD slot definitions Fabio Baltieri
2013-05-08  8:19   ` Lee Jones
2013-05-08  7:14 ` [PATCH 6/6] ASoC: ux500: Use the first two AD slots for capture Fabio Baltieri
2013-05-08  8:22   ` Lee Jones
2013-05-08 10:56   ` Mark Brown
2013-05-08 11:12     ` Lee Jones
2013-05-08 12:30       ` Mark Brown
2013-05-08 16:08     ` Fabio Baltieri

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=20130508120326.GF3459@gmail.com \
    --to=lee.jones@linaro.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=fabio.baltieri@linaro.org \
    --cc=lgirdwood@gmail.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ola.o.lilja@stericsson.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox