linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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



  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).