Xserver driver merging pros & cons

[Jeremy Huddleston]

On Sep 16, 2011, at 13:10, Stéphane Marchesin wrote:

> 2011/9/16 Jeremy Huddleston <jeremyhu at apple.com>:
>> 
>> On Sep 16, 2011, at 12:45 PM, Stéphane Marchesin wrote:
>> 
>>> Well, the issue is not separate. Driver repos are straightforward to
>>> get changes in and therefore can move forward easily.
>> 
>> How would that be different from an xorg-server tree with the exact same privileges?  I don't see any difference as far as individual contributors are concerned.
>> 
>>> For xserver changes, with the amount of latency/difficulty involved, people don't
>>> even try.
>> 
>> Can you be specific?  What latency are you talking about?
>> 
>> 1) Is it latency getting a [PULL] or a ready [PATCH] applied?  Perhaps we can address that.
>> 
> 
> The difficulty of getting reviews, the difficulty to attract attention
> to a patch and then get it pulled by someone who doesn't understand
> it.

I pull patches that I don't understand.  I just make sure it was reviewed by someone who does understand it.

>> 2) Is the latency involved due to missing reviews?  We need more people to step up and actually review.  Yes, this is a problem which we need to discuss.
> 
> Extending a model which is currently not working won't fix the model.

If the reason the model doesn't work is because people don't review, then some how getting people to step up and give reviews would fix the model.

> I would say we need to fix the model first

What is specifically wrong with the model?  So far the only thing you've stated concretely is that not enough people review patches.  We are aware of that?  Do you have suggestions?  Reviewing code is not the most fun thing, but we *ALL* need to do our part.  If you're going to submit a patch, review someone else's.

> then if that flies we can
> merge the drivers back. I haven't seen proof of the former happening
> so I cannot defend the latter.

This week is the first time I've heard that there is a problem.  Last week, I thought everything was working well aside from review latency.

>> 3) Is the latency involved due to reviews which come back with comments rather than Reviewed-by: tags that developer is lazy in responding to and thus don't bother because "my patch is good enough for me, why should I change it to appease reviewer X".  If this is the case, I think it's good that the patch wasn't committed but bad that it dropped on the floor.  Yes, I understand that it can feel like a waste of time to go back and edit a commit to respond to a reviewed, but doing so will make your commit BETTER and possibly prevent regression.
>> 
> 
> Well, who will review driver code? Only people who contributed to that
> driver before, and in that case you might as well just keep the
> current model.

Yes.  People who work on the driver review the code.  The maintainer of the driver sends a [PULL] request.  Excellent!  I fail to see the problem.

> My point is, for drivers, that review system will slow
> things down (because you need to wait for a pull from someone who has
> no familiarity with your code)

No you don't.  The code is already in your tree?  Why do you need to wait for it to land on master?  Why do you CARE when it lands on master?  The only people who care when it lands on master are release managers and distributions.  Driver developers won't even be using master, they'll be using their branch off of master.

> , and will not generate more relevant
> reviews (because the relevant reviewers are already working on the
> driver anyway).

I never said this had anything to do with getting more relevant reviews (although that is certainly possible).  I'm not even suggesting that you need to send your patches to xorg-devel; I'm just saying that the commits need to have a Reviewed-by: tag from someone who knows what they're talking about.

>> WRT driver contributors specifically:
>> 
>> There is nothing preventing the drivers from having their own xorg-server tree with the same commit privileges that currently exist for the drivers.  Everyone developing drivers will continue to push to that, and periodically, it will merge into master or a stable branch as appropriate (and the result will be merged back into it).  Individual contributors don't need to worry.  The owner/maintainer of the driver just needs to email the release manager with a [PULL] request every few weeks.
> 
> It's the theoretical model of the current xserver, and as shown in
> this thread it has slowed down changes tremendously since we rely on a
> few overloaded individuals for reviews/merges. I don't see how that
> would work better once you add drivers to that workload.

So *OFFER* suggestions.  We're all ears.  I don't consider myself overworked and usually respond to cherry pick and PULL requests to stable within a couple days.

> Until we see
> that model working as well as what we have previously, I don't think
> we should use it for more code.

Give me a metric that gauges "working" versus "not working" ...