Getting the right kind of contributions

Most free software projects encourage contributors—it is the rare project that has an overabundance—but contributions vary greatly in quality. Encouraging good submissions, or those likely to lead to useful contributions down the road, is an important part of any project. But it is a delicate balance. It can be difficult to determine the kinds of tasks suitable for new contributors that will lead to more important contributions later.

The flip side of that coin is how to handle contributions that appear to lead elsewhere. Just wading through the significant submissions on a large project's mailing list—linux-kernel being an excellent example—is extremely time consuming; adding noise, in the form of less-than-completely-useful patches, only makes that job harder. New contributors generally want to start with something relatively easy, though, which leads to the tension.

Discouraging patches that aren't particularly useful in a way that won't chase off prospective kernel hackers is hard. Al Viro's rather intemperate call for discussion of a linux-wanking mailing list on linux-kernel is probably not the right approach. He was responding to a patch that reformatted a kernel header file to line up the arguments. Viro is not known for his diplomatic skills, but he was responding to a problem that he and other kernel hackers see. There is an increasing amount of trivial cleanup work being submitted that is not translating to more substantial, useful contributions later on.

In a followup post, Viro explains his concern:

There is a real cost associated with posts to linux-kernel. It is the main communication mechanism for kernel development so those involved need to work through the posts there. David Miller laments the time he spends sorting through it all:

The kernel project provides a number of resources for people who are interested in getting involved but don't know where and how to start. The Kernel Newbies effort is specifically designed to help people get started with the kernel by running a wiki, mailing list, and IRC channel that are focused on the needs of, well, newbies. The idea is to provide information and mentoring that will lead to useful contributions to the kernel.

A subproject is the Kernel Janitors who focus on cleaning up kernel code:

Both of these efforts are targeted at getting people up to speed so that the kernel as a whole improves. All of the work is important, but there are many other kernel tasks that are not getting done, possibly because contributors are concentrating on cleanups. Andrew Morton has some suggestions for interested folks:

One problem, though, is that much of that work is more difficult than a whitespace cleanup. For those who are interested in getting their name "up in lights"—in the form of a kernel commit message—the trivial patch path appears easier. Responses like Viro's may deter them, but it risks making linux-kernel look like a hostile place that does not encourage new developers.

Some extremely important kernel tasks often do get little or no recognition. Submitting detailed bug reports, bisecting the kernel to find the patch that broke things, or testing proposed fixes go unrecognized—at least in the kernel commit log. There have been thoughts of adding tags to the patches that would note these contributions, but no concrete proposal has been made.

Two other documentation efforts are underway to assist new kernel developers. Jesper Juhl is working on a Kernel Newbies Guide to be included into the kernel Documentation tree. It may get folded into Documentation/HOWTO or as a separate file, but the idea is to steer folks in the right direction—and away from the kinds of patches that raise the ire of kernel hackers. LWN's Jonathan Corbet also mentioned a longer document he is working on with support from the Linux Foundation that should be ready for review in June.

There may be some rudeness or hostility towards new developers on linux-kernel, but it rarely rises to the level seen on the openbsd-misc mailing list last October. In response to a query about a list of less complicated tasks for OpenBSD—similar to what the Linux Kernel Janitors maintain—project leader Theo de Raadt, who is really not noted for his diplomacy, blasts:

This is sort of the extreme end of the "show us the code" attitude, but, in his own inimitable way, de Raadt is reacting to the same problem. It takes time and effort to shepherd new kernel hackers. Spending time mentoring folks who will never end up contributing is a waste; that time is better spent finding, fixing, or adding bugs. As Linux hacker Ted Ts'o puts it:

How does a project determine which newly interested people will end up being useful contributors versus those that will not? It is a difficult problem that warrants some thought. It surely isn't just kernel projects that have it, as any large, high-profile project will have both a fairly high barrier to entry along with some developers who should be discouraged.

Obviously there will never be a clear-cut "future contributor" test, but there may be ways to get a better idea. In the meantime, flaming well-meaning folks to a crisp is unlikely to get there. Referring inappropriate patches to Linux Newbies or something similar—on the off chance the person can be redirected—might be a start.

I don't know the answer.  But I do know that a dev team so antagonistic to potential
contributors bodes ill.  This looks quite similar to the "Linus Crisis" which led to
BitKeeper, and eventually to git.  Except that this time it is not just Linus who is
overloaded.

Dumping large amounts of complex information into an unstructured medium like a mailing list
is, perhaps, not the best solution.

Thank you!  That link quickly lead me to announcement of the new list
<linux-embedded@vger.kernel.org>, to which I am now happily subscribed.

Perhaps there was an announcement on linux-kernel.

But I burned out on linux-kernel a few years ago: simply too much traffic to have any energy
left after keeping up with it, even skimming for the bits relevant to me.  I was sad to be out
of the loop at first, but the increased energy for other things soon made up for it.  I've
always been amazed at folks who seem to keep up with linux-kernel _and_ have time/energy to
actually do kernel development.

I fully appreciate the value of diplomacy, but I didn't think Viro's comment was all that
offensive.  It's clearly a joke as nobody would permit a "wanking" list.  As the article
points out, wander over to OpenBSD to see how far from diplomatic you may be treated.

We've all seen comment threads here on LWN that have been a little less than diplomatic.  Yet
kernel devs still post here, a wide diversity of opinion still exists, and people (so far as I
know) aren't turned away.

If I may play devil's advocate, wouldn't you agree that to be a decent contributor you'd have
to be involved with the kernel community?  And that being involved would inevitably require
reading a fair portion of LKML?  Anyone willing to send a patch to LKML should be familiar
with how LKML operates -- and therefore shouldn't be surprised at the reaction to an unwelcome
patch. 

Your point is valid, in that Asian cultures might be more deterred than others.  It would be
difficult to quantify who was offended and therefore didn't participate.

I've often seen that diabolical argument used to justify the relative lack of participation
from anyone other than mostly young, introverted, Caucasian, middle-class males.  There's
definitely something to the argument that people unwilling to participate won't participate,
but encouraging artificial barriers to participation from people who might otherwise
participate seems counterproductive.

If I remember correctly, the story was about an embedded system maker who modified the Linux
kernel to support its system but didn't submit the patches because didn't want to put up with
flames.

So why aren't these trivial patches being referred to Kernel Janitors?

"""
So why aren't these trivial patches being referred to Kernel Janitors?
"""

That is a good question.  But I will point out that "trivial" is quite a relative thing.
"Coding Style" was a significant theme when devfs was being battled out on the list.  And as I
recall, Al didn't think that it was trivial when he was beating Richard Gooch over the head
with it.  Why now?

Coding style is one thing. *Alignment of parentheses* is another.

(And yes, when I'm maintaining other people's code I do realign them if 
they're misaligned and I'm working in that area: but submitting patches 
that do nothing else is barmy.)

As the matter of fact, devfs *had* multiline argument lists aligned.
Which had zero impact on readability...

There's one thing sadly missing in all that mess, as you've just shown.
Common sense.  Note that with all the rules in Documentation/CodingStyle,
the common sense is not even mentioned there anymore...

One would hope that common sense is kind of assumed.

(OK, OK, I know...)

Two examples to look at: security advisories and newspaper bylines.

Vendor security advisories are good at crediting the person who reported a security bug -- why
not something similar for anyone who runs a "git bisect" session that results in a fix, or
builds and tests a patch? 

The old-school newspaper rule is that if you see a byline of "Mary Example and Joe Random"
then Mary was the one on-scene telling about the events to Joe, who actually wrote the copy.
Maybe the kernel could do something similar for lengthy troubleshooting sessions.

(And credit all whitespace fixes to whitespace-wanker@kernel.org and all spelling fixes to
spelling-wanker@kernel.org ?)

There is of course a major difference between badly written code and trivial white spaces even
though CodingStyle can cover both of them. In addition to that, it might be a valid thing to
address in a review of new patches but drowning the list in such patches to known working code
is asking for trouble. 

It seems like the best approach would be to have a path (through kernel newbies and/or kernel
janitors and thence through a small group of volunteer liaisons to the main kernel list.

Then all the clean-up patches can be consolidated, reviewed tested and merged in a streamlined
manner.

Perhaps one technique could be for new (substantive) patches to go through LKML -- then to
testing -- then through the janitorial process (which adds the stylistic and cosmetic
clean-up) --- then back through testing (-mm or similar kernels) and finally back through LKML
on its way to main.

In that model the janitorial team keeps their clean-up patches in their own repository, in
holding, and merges the clean-ups with substantive patches as those arise.  Occassionally they
inject a separate patch to a messy but long  untouched area of the sources --- but it's all
coming from them and done in a way that doesn't burn out the LKML participants.  (So such
clean-up only injections can be done opportunistically when the list has been relatively quiet
for a few days).

Just a thought.

JimD

I think that is the best way to manage contributions (of all sizes). I think the Kernel
Janitors and the "Trivial Patch Monkey" maintain some patchsets, but perhaps there should be
more official git trees. For example:

linux-whitespace
linux-trivial
linux-warnings
linux-janitors

could be maintained separately and merged into linux-testing or -mm at convenient merge times.

Perhaps.

But there's also the point that sometimes two patches:

1) This patch does NOTHING other than reformat function foo()
2) This patch adds a check for $ERROR_CONDITION to foo()

Is easier to read and understand the patches than it would've been if they where rolled into
one.

It's not that it's hard. It's just that -any- noise subtracts from readability.

Which means a patch that -only- does the minimum required for adding the error-check will
always be more readable than one that does that, plus fix the formatting of the surrounding 5
lines.

So your advice adds up to, essentially: NEVER FIX STYLE-ISSUES.

Which is overdoing it. I agree that it's *often* a bad thing to do style-change. But there's
two important readability-issues here:

One is the readability of the patch. That is always better with zero style-changes.

The second is the readability of the code after the patch is applied. If that is sufficiently
improved by doing style-fixes, those may be worthwhile even though it hurts the patch.

Not sure about the problem itself, but I sure loved the Al Viro's initial letter along with
the attached Exhibit A.

We obviously need a programming language in which there is only one valid way to write a given
sequence of tokens, and all other inputs generate compiler errors.

who's preferred layout would everyone agree to use?

Python is trying to do this, but while people who like it tend to really love it, many others
dispise it for the same reasons.

Linux already has an "official" CodyingStyle (including indent configuration) and a checkpatch
script. Perhaps every checkin must be run through indent before acceptance. And to get
everyone up to date with the same baseline indent style, have a flag day where Linus' git tree
gets indented (to fix any currently offending files).

the initial post was suggesting that someone write a language that enforced the coding style
so that any deviations would result in a compile-time error.

that's a far cry from the kernel coding style document (which by the way, is a guideline, not
a hard requirement. see the flame-wars over checkpatch)

the problem with a flag-day reformat is that it will break everyone's patches that haven't
been merged yet.

the only possible exception to this is Ted suggested doing a flag-day whitespace cleanup, and
modify tools to ignore whitespace changes on the - lines of patches to avoid this
incompatibility.

but other reformatting needs to include a judgement call, not just mechanical formatting

That's actually one of the problems in some areas - patches get knocked back due to violating
CodingStyle but being consistent with the rest of the file they touch. Without going through
and improving the entire file the patch introduces another form of bad practice.

It seems to me that what is needed is a smarter diff tool - one that parses the code the same
way the precompiler and compiler do, and doesn't bother us poor humans with the noise
generated by simple whitespace changes.

Yeah, but how do you apply patches in the presence of intervening 
whitespace changes? A `smarter patch' in that sense would need to have a 
language-specific understanding of indentation, at the *very* least...

I don't think this would be a problem in C, since indentation doesn't have semantic meaning.

Try geting in after someone has de-K&Red your project and applying a patch 
generated earlier, and oopsy. Any line-based patch system would be 
confused if { and } had moved from lines with code on them to lines 
without, or vice versa.

Maybe keep stupid patch (line-by-line) but have smart diff that understands the language and
parses by tokens not lines? Patchfiles would still be noisy, but humans would ignore that and
look at output of smart diff to see what had really changed.

But that doesn't fix your pre- and post- de-K&R problem does it? Patch would have to be smart
(token, not line-based) too.

Yeah, a line-based patch system wouldn't work in that case.  But I think lexically scanning
the input would be sufficient -- you wouldn't have to parse it.  If two files generate the
same sequence of tokens, then I think they could be considered equivalent in a "free form"
language.

You are missing the difference between "trivial patch" and "pointless
doodling".  The former can be quite useful, both in gaining experience
and in improving the tree.  The latter is not and no, dumping that on
kernel-newbies is not a solution - it would only move the thing to the
place where it could do most damage...

take the Linus quote in context. he was saying that it's not the maintainers job to write
code, but to make sure that the code that's accepted is the right thing to do.

he was mostly talking about the functionality, not submission process or code style.

The major strength of the Open Source development model is that people contribute to get a
problem fixed that *they* have. I believe that most contributors to the linux kernel fix
problems they have or work on features they need (privately or as part of their job).
Who needs whitespace-correct code? Only people who need to work with the source code later on.
I wonder how long the "I want to get involved in Linux kernel development because it's cool"
motivation can last. But the "I have this (e. g. embedded) system and I get this OOPS every
*third* time I do *that*" motivation can last until the problem is fixed in the mainline
kernel.
I believe that helping newbies who report errors or strange behaviour and showing them how
they can find out what is wrong will eventually lead to interesting contributions, much more
that discussion on how to deal with whitespace patches.

Hartmut