Re: [PATCH v8 2/3] drm/panel: Add support for S6E3HA2 panel driver on TM2 board

From: Thierry Reding
Date: Thu Feb 02 2017 - 11:49:06 EST


On Thu, Feb 02, 2017 at 05:30:34PM +0200, Jani Nikula wrote:
> On Tue, 31 Jan 2017, Thierry Reding <thierry.reding@xxxxxxxxx> wrote:
> > On Tue, Jan 31, 2017 at 10:15:10AM -0800, Eric Anholt wrote:
> >> I would love for drm-panel to be moved under -misc.
> >
> > Like that's going to magically motivate people to spend their time
> > reviewing other patches. The only thing that group maintainership adds
> > is redundancy.
>
> Adding redundancy is not an insignificant thing. I think it can be quite
> liberating to not have everything and everybody depend on you. You can
> defer to others when you're busy, tired, sick, whatever. Things still
> move on.

Oh, I certainly see the advantages in sharing maintainership. However, I
think it really only works well if you've got active developers working
together on the code already. Then it's very natural to let all of them
manage a tree.

But for drm-panel, I've rarely seen anyone review patches. Sometimes the
easy patches (i.e. panel-simple) get reviewed by parties that have some
interest in seeing the patches merged. However, there's usually no
review at all for the more complicated patches.

Given that, I doubt that group maintainership is going to have the
desired effect. Just because the tree is group maintained doesn't mean
that all of a sudden people are going to care about the patches. So I
suspect that even if panel drivers were managed in drm-misc, I'd still
be the only person reviewing the patches. And really, managing a tree
is peanuts compared to the amount of work it takes to properly review
code.

With group maintainership, for this type of tree in particular, I think
it's likely for the quality bar to be lowered. There aren't any best
practices yet for anything beyond panel-simple, and therefore little to
no review is likely going to make people still pick up patches. This is
quite different to the DRM/KMS core bits and drivers because there are
established best practices and people can usually spot when new code is
not living up to expectations.

> And I don't think redundancy is the only thing that group maintainership
> adds. You'll have maintainers that complement each other, with different
> skill sets and abilities and experience. They don't all look at the same
> things. As maintainers tend to be more senior folks, I find sharing the
> load of the more mundane tasks of maintainership free up their time to
> contribute more of their technical skills to the project, for example
> review.

I don't understand why group maintainership would be necessary for this.
Surely people will review code irrespective of who finally applies their
patch. If they don't in a single maintainer project, why would they
suddenly start reviewing code in group maintained projects?

> It's just my personal view on i915, but I think people take more
> responsibility of their own work, instead of just sending patches and
> waiting for stuff to happen, when they have commit access. But you have
> to trust the people.

That's not something that we're discussing here. Surely giving the world
access to the maintainer trees is not a goal that we're pursuing here.
It will still only be a handful of selected people that will have commit
access, so how's that going to change things for contributors that don't
have commit access?

The bottom line still is that we have a requirement to have patches
reviewed before they get applied. So even if everybody had commit access
we'd still need someone to review a patch before it gets applied. Like I
said, reviewing is really the difficult part of a maintainer's job.
Applying a patch is trivial, build-testing is equally trivial and so is
sending out a pull request. All of the above can be easily automated to
a point where it's completely painless.

> I didn't intend for this to become a kind of sales pitch, but I do think
> drm-panel would be a good fit for drm-misc. Personally I think it's your
> call, but I think you should think about it. (And that decision should,
> obviously, be made calmly, independent of any particular patch series.)

You know what? I completely agree with you that drm-panel would be a
good fit for drm-misc. It's effectively part of the DRM/KMS core and
used by most drivers. But it's never been treated that way. Maybe it
is because it's been maintained in a separate tree from the very
beginning, perhaps that was a mistake in the first place. Then again,
back when drm-panel was started we didn't have group maintainership,
so none of the existing trees would've been a good fit.

In the end, I keep getting back to the question that everybody except
me seems to know the answer to: why would there be a difference in how
contributors behave depending on the number of maintainers?

Thierry

Attachment: signature.asc
Description: PGP signature