[Ksummit-discuss] [CORE TOPIC] Recruitment (Reviewers, Testers, Maintainers, Hobbyists)
Rafael J. Wysocki
rjw at rjwysocki.net
Mon Jul 20 22:35:08 UTC 2015
On Monday, July 20, 2015 10:58:07 AM James Bottomley wrote:
> On Mon, 2015-07-20 at 01:48 -0700, Josh Triplett wrote:
> > On Mon, Jul 20, 2015 at 08:15:15AM +0100, James Bottomley wrote:
> > > On Sun, 2015-07-19 at 22:16 -0700, Josh Triplett wrote:
> > > > On Mon, Jul 20, 2015 at 10:34:35AM +0800, Zefan Li wrote:
> > > > > > I.e. I might propose a a slightly controversial topic, going a bit the
> > > > > > other direction than the whole "motivating newcomers" discussion: how to
> > > > > > get rid of useless submissions that are slowing maintainers down?
> > > > > >
> > > > >
> > > > > Do we really have this issue?
> > > > >
> > > > > If we are encouraging more people to get involved in kernel contribution, we'll
> > > > > sure occasionally see some patches with little value, but I don't think we are
> > > > > suffering from this.
> > > > >
> > > > > And When we see a patch of this kind, it won't take us much time to tell the
> > > > > newbie why the patch isn't appropriate, and then he probably won't do this again.
> > > >
> > > > That's exactly the kind of thing that we *shouldn't* do.
> > > >
> > > > Think about that from the new contributor's perspective. They've made a
> > > > change to the kernel that has a small but non-zero value. They've just
> > > > managed to work out how to jump through all the hoops needed to prepare
> > > > and submit it properly for the kernel, through some combination of
> > > > reading, lurking, and mentorship. And the first response they see is a
> > > > maintainer like you saying "please don't send this kind of patch".
> > > >
> > > > Yeah, they probably won't do that again. Congratulations, you defeated
> > > > the newbie and thwarted their evil maintainer-time-wasting scheme! Hail
> > > > the conquering hero; insert victory fanfare here. If you and others
> > > > keep up that vigilance, perhaps one day all maintainers can rest easy,
> > > > knowing they'll never again have to deal with new contributors.
> > > >
> > > > </sarcasm>
> > > >
> > > > It's perfectly reasonable to tell someone that, since they've gotten the
> > > > hang of sending kernel patches, they should move on to more substantial
> > > > changes, and leave simpler fixes to other potential new contributors.
> > > > But that's different than telling them their patch is unwelcome.
> > > >
> > > > (If someone sends in a patch that's actively wrong, sure, go right ahead
> > > > and tell them what's wrong with it. But there's a difference between
> > > > "wrong" and just "not that important".)
> > >
> > > I think that's the wrong attitude in so many ways. Good teachers don't
> > > accept crap.
> >
> > We don't seem to be talking about the same kind of patches, then. If
> > someone sends in incorrect patches, by all means reject those patches.
> > But a patch that improves code, even a very minor improvement, should
> > always be welcome.
>
> "Improvement" is probably the issue. Improvement to me means
>
> 1. Adds a new user visible feature that will have consumers
> 2. makes a user visible change that adds value (ideally a bug fix,
> but I think adding extra debug or other interfaces can count
> here)
> 3. materially improves the maintainability of the code.
>
> The third one seems to be where there's most disagreement. Usually the
> guideline I use for this is that for older files is just don't touch.
> If someone really wants to improve the file then they get to maintain it
> as well (we've had some success with this) otherwise generally such
> patches aren't welcome.
Agreed.
> > (This doesn't mean that mechanically fixing compiler warnings, for
> > instance, is always an improvement. For instance, shutting up the
> > compiler rather than actually fixing the warning is not a good idea.
> > But when the patch actually fixes something, even something minor, it's
> > worth accepting.)
>
> I don't agree that all improvements, however minor are worthwhile.
> There's a cost to reviewing and merging ... that should be outweighed by
> the value of the contribution.
Right. Plus a change making an old file generate less checkpatch.pl warnings
may actually *not* be an improvement (even if it doesn't break things actively).
There is code in the kernel that was written with a coding style quite different
from the one regarded as a "standard" today and there's nothing wrong with that
in principle.
Thanks,
Rafael
More information about the Ksummit-discuss
mailing list