Re: 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]

 



Em Thu, 11 Jun 2009 08:23:37 +0200 (CEST)
Guennadi Liakhovetski <g.liakhovetski@xxxxxx> escreveu:

> Hi Mauro, thanks for replying and for the explanation. I'll skip most of 
> your message, and just keep the bits I'd like to reply to.
> 
> On Wed, 10 Jun 2009, Mauro Carvalho Chehab wrote:
> 
> > 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.
> 
> Hm, I am not sure, is Greg's quilt tree publically available? And how many 
> actually use it? Whereas your hg tree is publically available and it looks 
> like a few do use it.

Well, he used to make it publicly available.

> > 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%. 
> 
> I still don't take this argument of increased traffic - I haven't seen a 
> single complain "please, don't increase the traffic, it'll make it worse 
> for me."

That's just what Hermann said.

> > 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.
> 
> I'm really happy we have subsistem maintainers that are such profecient in 
> their work and such confident in their results that they don't need any 
> reviews and discussions. I for one is not one of them, that's why I always 
> first send my patches to the list.
> 
> > Also, for the developer, using the pull request is better, since they can
> > better track when a patch series got merged.
> 
> I never argumented against pull requests, I'm suggesting they should 
> follow patches posted to the list.
> 
> > 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.
> 
> Yes, I see what you mean, but 1) you cannot avoid it, there are always 
> patches from various authors, that they send to the ML, that some 
> driver-maintainer will then pull through his or her tree and ask you to 
> pull it. So, we really have to learn to proces this case efficiently.

The current process were built along the time and, up to now, it is the better
we have. For sure it can be improved, but we need to take care to not transform
it into some bureaucratic set of procedures that reduces our development speed
and doesn't aggregate any value.

> > 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
> 
> Well, I guess, I will have to subscribe to that hg-commit list (or 
> whatever it's called), and use it. The problem is - it is a bit too late. 
> But it's the best option available so far.

It is not that late, and you can always reply over the PULL requests, when you find an issue there.

> Another question, if you pull patches from someone's tree for review of 
> one of those pull-requests (as you described in this mail, but I've 
> deleted that piece already), how do you then quote the code if you want to 
> comment on it? You export the patch again, hit reply on the pull-request, 
> and paste the patch into it? And then add the quotation marks "> " 
> manually?...

Well, I generally do:

	./hgimport <tree>

then I check each patch at /tmp/newpatches with less (or with kdiff3, if the patch is very complex).

If I need to comment about a patch, I simply do:

	cat /tmp/newpatches/hg_v4l-dvb_1.patch|perl -ne 'print "> $_"'|xclip

and then I paste it at the email with a CTRL-V.

The result is something like this:

> 
> # HG changeset patch
> # User Andy Walls <awalls@xxxxxxxxx>
> # Date 1243037266 14400
> # Node ID 9fb907f46b2aa301a2f270832e8cdcbc82ac8744
> # Parent 315bc4b65b4f527c4f9bc4fe3290e10f07975437
> lnbp21: Add missing newline
> 
> From: Andy Walls <awalls@xxxxxxxxx>
> 
> Reported-by: VDR User <user.vdr@xxxxxxxxx>
> 
> Priority: normal
> 
> Signed-off-by: Andy Walls <awalls@xxxxxxxxx>
> 
> --- a/linux/drivers/media/dvb/frontends/lnbp21.c	Sun May 17 12:28:55 2009 +0000
> +++ b/linux/drivers/media/dvb/frontends/lnbp21.c	Fri May 22 20:07:46 2009 -0400
> @@ -133,7 +133,7 @@ static struct dvb_frontend *lnbx2x_attac
>  	/* override frontend ops */
>  	fe->ops.set_voltage = lnbp21_set_voltage;
>  	fe->ops.enable_high_lnb_voltage = lnbp21_enable_high_lnb_voltage;
> -	printk(KERN_INFO "LNBx2x attached on addr=%x", lnbp21->i2c_addr);
> +	printk(KERN_INFO "LNBx2x attached on addr=%x\n", lnbp21->i2c_addr);
>  
>  	return fe;
>  }
> 

Then I delete the lines I don't need, to avoid polluting the email with non-relevant stuff.



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