pull requests x patches at linux-media ML - Was: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



First of all, I'm renaming the thread, since we're not discussing the pull
request per se. So, let's create a separate thread for it.

Em Wed, 10 Jun 2009 23:18:52 +0200
hermann pitton <hermann-pitton@xxxxxxxx> escreveu:

> Hi,
> 
> Am Mittwoch, den 10.06.2009, 22:39 +0200 schrieb Hans Verkuil:
> > On Wednesday 10 June 2009 22:20:03 Guennadi Liakhovetski wrote:
> > > On Wed, 10 Jun 2009, Hans Verkuil wrote:
> > > > On Wednesday 10 June 2009 20:17:53 Guennadi Liakhovetski wrote:
> > > > > On Tue, 9 Jun 2009, Hans Verkuil wrote:
> > > > > > Hi Mauro,
> > > > > >
> > > > > > Please pull from
> > > > > > http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2 for the
> > > > > > following:
> > > > > >
> > > > > > - v4l2: add new s_config subdev ops and
> > > > > > v4l2_i2c_new_subdev_cfg/board calls - v4l2-device: fix incorrect
> > > > > > kernel check
> > > > > > - v4l-compat: add I2C_ADDRS macro.
> > > > > > - v4l2: update framework documentation.
> > > > > > - v4l2-subdev: remove unnecessary check
> > > > >
> > > > > Do I understand this right, that these patches have not been posted
> > > > > to the list?
> > > >
> > > > The idea is that you click on the link and look at the patches through
> > > > the hg web frontend.
> > >
> > > And then?
> > >
> > > > > At least I haven't found them in online and in my local
> > > > > archives. If it's really so, sorry, this doesn't seem very productive
> > > > > to me... We have discussed this with Mauro on IRC, he didn't agree
> > > > > with me, he thought it was acceptable in many cases... Sorry, cannot
> > > > > agree.
> > > >
> > > > Both methods (a pull request or a patch series) are used and personally
> > > > I have no preference, although currently I have a script that
> > > > simplifies these pull requests so I generally use that. A patch series
> > > > makes it easier to reply with review comments, while I think a pull
> > > > request reduces mailinglist traffic and actually makes it easier to do
> > > > the actual reviews.
> > >
> > > I think, patches posted to the list for reviews with a following pull
> > > request (if no rework needed of course) is the most reviewer-friendly,
> > > which is also what I so far have seen on all other lists I subscribe to
> > > (arm, ppc, usb, scsi, lkml, i2c,...).

On LKML, several patches are sent as pull requests.

> > >  Have you seen those huge mailing
> > > threads from Greg K-H with all patches in his queue preceding his pull
> > > requests (I hope I reproduce his work flow correctly here, any mistakes
> > > are mine and unintended)?

The same happens here: All patches added at the staging tree are sent to
linuxtv-commits ML. So, they are there for discussions before my pull requests.

The main difference is that, in the case of Greg, his staging tree is a quilt
one. On our case, our staging tree is mercurial.

> > > Are you really saying that it's equally convenient for you to review /
> > > reply to patch on ML and to patch in some repository from a pull request?
> > > Especially when there are multiple patches in that pull and you have to
> > > click through them all, jumping back and forth between your mail client
> > > and a browser?...
> > >
> > > If all are so much concerned about the ML traffic (which I don't
> > > understand either, filtering and ignoring uninteresting mails is easy
> > > enough these days), maybe we should split into devel and user? Sorry, I
> > > really don't understand. I'll go ask members of other MLs what they think
> > > about "clicking" through patches...
> > 
> > Um, you are asking the wrong person. It's one of the two methods used on 
> > this list. Yes, pull requests are unusual compared to other lists (and so 
> > is the use of hg instead of git for that matter due to historical reasons).
> > 
> > If Mauro says: use patch series, then I'll modify my workflow. If you prefer 
> > to see these subdev patches as a patch series, then I can do that for you. 
> > I have no preference myself. It's also a discussion I have no wish to go 
> > into.
> > 
> > So if you see a pull request from me and prefer to have it as a patch 
> > series, just mail me and I'll do it. No problem.
> > 
> > Regards,
> > 
> > 	Hans
> 
> on the pull requests is at least nothing new since years.
> 
> Previously all patches were on video4linux and the linux-dvb ML and
> dealt with independently as far as possible.
> 
> Because of all the hybrid devices that changed, but still someone having
> only analog TV reception likely doesn't want to read all about the
> digital stuff and in the other direction I assume in counts even more.
> 
> So far the mercurial pull requests from the more active developers
> worked quite well. Historically seen you would have had a need at some
> point to see _all_ patches on both lists, if you follow the rule _all_
> patches must be on the list(s).
> 
> Now, with linux-media, everybody subscribed has the traffic of both of
> the old lists. Means for most people 50% are off topic.
> 
> But the really funny thing comes now, we have with you and all the
> others suddenly about 70% of traffic on the list about cams :)
> 
> I'm sure that more than 90% of the old v4l and dvb list members are not
> interested in that stuff at all :)

We're currently merging about 900 patches per kernel window, on a window of
about 10-12 weeks. This means about 90 patches per week, or about 13 patches
per day (for a 7 days week), or 18 patches per day (for a 5 days week).

So, if people just send one email per patch, this will increase our traffic by
18 emails by day. It can be worse than that, if we consider that patches can be
replied, and that people use to write a patch 0 to describe a patch series.

Considering about 50 messages per day, currently (today and yesterday's
statistics - not sure those are typical days), this would increase the ML by
about 36%. 

On the other hand, part of those patches are already discussed previously at
the ML (on today's case, none of the patches sent were RFC patches and patches
that were resent - so, we would add 36% of traffic today, plus the extra patch
0 and comments - so, it would have increased it by something like 40% to 50%).

We should also consider the type of patches we have here:

1) trivial patches (typo fixes, coding style, simple board additions, Kbuild
fixes, etc);

2) bug fix patches at drivers;

3) new drivers;

4) core changes.

However, several driver maintainers don't care (or just forget about they) for (1)
type. Before patchwork, lots of such patches were lost forever in the middle of
dvb and v4l mailing lists. They are happy when someone (me) get those patches
and apply at the tree or remind they to check and apply on their trees.

patches of type (2) and (3) are in general sent via a driver maintainer and
generally doesn't generate discussions.

>From my previous experiences, even patches (1), (2) and (3) are reviewed when a
pull request is sent. That's one of the reasons why I sometimes wait for some
time before committing. Especially on the cases where the patches are more
complex, bigger or are likely to cause more discussions, I tend to wait more
before merging they.

I have no doubts that patches of type (4) need previous discussions. However,
even in this case, in general, the developers that propose such patches tend to
previously discuss they with the others at the ML and at IRC. For example, in
this specific case, at the first version of the pull request, Hans said:

	"I would prefer to see this go into 2.6.31 since this will help the 
	 development of embedded v4l drivers, but if you prefer to delay this until 
	 the 2.6.31 window closes, then that is OK by me. But I do not know if that 
	 is also OK by Guennadi, Eduardo and Karicheri. All three (and probably 
	 others I missed as well) are waiting for this functionality."

So, I understood that Hans, Guennadi, Eduardo and Karicheri are all OK with this
approach. Yet, I agree that a wider discussion should be done.

That's said, I can't see much ways to improve this situation. 

While patchwork actually helped a lot to avoid missing a patch, it has some
limitations. Due to that, instead of just reading a patch at my patchwork queue
(that I get via a xmlrpc script), I need also to read the patchwork thread and,
on some cases, follow the patches thread at the ML. So, instead of increasing
my merge speed, on several cases, it just add more work. On the other hand, on
most cases, a pull request is a way faster.

Also, for the developer, using the pull request is better, since they can
better track when a patch series got merged.

The usage of a mix of PULL and PATCH requests has an extra trouble: it means
that we'll receive most of the patches duplicated. So, it means that I need to
manually mark all merged patches at patchwork queue, on _each_ pull request.

So, this adds an extra cost that will probably make life worse for everybody,
with almost no gain (since there are really very few complaints about badly
merged patches).

That's said, I'm open to listen to opinions on how to improve our current process



Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux