From: Larry Finger <Larry.Finger@lwfinger.net>
To: Kalle Valo <kvalo@codeaurora.org>, linux-wireless@vger.kernel.org
Subject: Re: wireless-drivers: random cleanup patches piling up
Date: Thu, 21 Jan 2016 13:46:15 -0600 [thread overview]
Message-ID: <56A13587.7040809@lwfinger.net> (raw)
In-Reply-To: <87wpr3x9ln.fsf@kamboji.qca.qualcomm.com>
On 01/21/2016 08:58 AM, Kalle Valo wrote:
> Hi,
>
> I have quite a lot of random cleanup patches from new developers waiting
> in my queue:
>
> https://patchwork.kernel.org/project/linux-wireless/list/?state=10&delegate=25621&order=date
>
> (Not all of them are cleanup patches, there are also few patches
> deferred due to other reasons, but you get the idea.)
>
> These cleanup patches usually take quite a lot of my time and I'm
> starting to doubt the benefit, compared to the time needed to dig
> through them and figuring out what to apply. And this is of course time
> away from other patches, so it's slowing down "real" development.
>
> I really don't know what to do. Part of me is saying that I just should
> drop them unless it's reviewed by a more experienced developer but on
> the other hand this is a good way get new developers onboard.
>
> What others think? Are these kind of patches useful?
Kalle,
We might get new developers, but the cost may be high. In the staging tree,
things are worse. The tools can be applied in a blind fashion, but the results
can be really stupid. GregKH has told a few would-be contributors to "go away"
after a few patches that would not build.
As most of these patches are based on "problems" found by application of various
standard tools, they will likely be resubmitted over and over until the code is
"fixed". Whether the patches are useful may not be the main question.
My real complaint with these patches is that very few are more than compile
tested. For example, there are 3 patches for memory leaks in b43. One
(https://patchwork.kernel.org/patch/7998941) was rejected because it missed some
such leaks, but there was not a formal NACK. The patch was fixed and resubmitted
(https://patchwork.kernel.org/patch/8014311/), but not yet tested. The author
then resent it (https://patchwork.kernel.org/patch/8049041/) and was chastised
for resending as it still had not been tested. Of course, the first two of these
can be dropped. Unfortunately, there are very few devs who have the necessary
hardware to test.
Most of the current set do not account for the directory restructuring and will
not apply.Those can be rejected with the appropriate message asking that they be
rebased. That should not require too much of your time. That will at last clean
out the current backlog.
Is it possible for us to require the patch author to supply the level of testing
when that is not obvious? This information should be in the comments location
after the first ---. I suspect I know the answer for non-maintainers, but the
formal requirement might be helpful. I will start NACKing those patches without
such information.
I also promise to be more diligent in reviewing the patches that are directed at
the drivers that I maintain.
Larry
next prev parent reply other threads:[~2016-01-21 19:46 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-21 14:58 wireless-drivers: random cleanup patches piling up Kalle Valo
2016-01-21 19:46 ` Larry Finger [this message]
2016-01-22 12:11 ` Kalle Valo
2016-01-21 22:32 ` Julian Calaby
2016-01-22 12:17 ` Kalle Valo
2016-01-22 13:13 ` Julian Calaby
2016-01-22 0:52 ` Joe Perches
2016-01-22 7:30 ` Dan Carpenter
2016-01-22 12:21 ` Kalle Valo
2016-01-22 15:12 ` John W. Linville
2016-01-22 15:54 ` Kalle Valo
2016-01-26 5:28 ` Sudip Mukherjee
2016-01-29 8:08 ` Kalle Valo
2016-02-01 4:41 ` Sudip Mukherjee
2016-02-01 8:21 ` Kalle Valo
2016-03-16 0:57 ` Julian Calaby
2016-03-16 9:22 ` Kalle Valo
2016-03-16 9:42 ` Julian Calaby
2016-03-18 1:06 ` Julian Calaby
2016-01-22 18:05 ` Joe Perches
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56A13587.7040809@lwfinger.net \
--to=larry.finger@lwfinger.net \
--cc=kvalo@codeaurora.org \
--cc=linux-wireless@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).