mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Adrian Bunk <bunk@kernel.org>
To: Roland Dreier <rdreier@cisco.com>, Linus Torvalds <torvalds@osdl.org>
Cc: Glenn Streiff <gstreiff@NetEffect.com>,
	Faisal Latif <flatif@NetEffect.com>,
	linux-kernel@vger.kernel.org, general@lists.openfabrics.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Greg Kroah-Hartman <greg@kroah.com>
Subject: Merging of completely unreviewed drivers
Date: Thu, 21 Feb 2008 23:01:24 +0200	[thread overview]
Message-ID: <20080221210124.GD28328@cs181133002.pp.htv.fi> (raw)
In-Reply-To: <adahcg2awag.fsf@cisco.com>

[ Linus Added to the To: since I want to hear his opinion on this issue. ]

On Thu, Feb 21, 2008 at 12:28:55PM -0800, Roland Dreier wrote:
>  > This driver should really have gotten some review before being included 
>  > in the kernel.
> 
>  > Even a simple checkpatch run finds more than > 250 stylistic errors
>  > (not code bugs but cases where the driver violates the standard code 
>  > formatting rules of kernel code).
> 
> Linus has strongly stated that we should merge hardware drivers early,
> and I agree: although the nes driver clearly needs more work, there's
> no advantage to users with the hardware in forcing them to wait for
> 2.6.26 to merge the driver, since they'll just have to patch the
> grungy code in themselves anyway.  And by merging the driver early, we
> get fixed up for any tree-wide changes and allow janitors to help with
> the cleanup.

Is it really intended to merge drivers without _any_ kind of review?

This driver even lacks a basic "please fix the > 250 checkpatch errors" [1]
and similar low hanging fruits that could easily be spotted and then 
fixed by the submitter within a short amount of time.

I see the point that it might make sense to not prevent the merging of 
drivers infinitely when they have some hard-to-fix issues, but was this 
really meant as an excuse for maintainers to no longer any review of 
what they merge at all?

> (By the way, the code is not that pretty but it a lot closer to
> upstream style than most driver submissions)
>...

There might be worse code being submitted, but when looking at what gets 
merged into Linus' tree this driver beats all other drivers I remember 
in both number of stylistic problems and bugs. [2]

>  - R.

cu
Adrian

BTW: Greg, you are Cc'ed for your joke in [3]...

[1] not to mention the > 2000 checkpatch warnings
[2] as already said, that's not meant against the driver submitter
    I'm complaining about the complete lack of review that would have 
    brought this driver into shape
[3] http://lkml.org/lkml/2008/2/12/427

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


  reply	other threads:[~2008-02-21 21:02 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-19 22:59 [2.6 patch] infiniband/hw/nes/nes_verbs.c: fix off-by-one Adrian Bunk
2008-02-20  4:23 ` [ofa-general] " Roland Dreier
2008-02-20  5:57   ` Adrian Bunk
2008-02-20 23:21     ` Roland Dreier
2008-02-20 23:27       ` Glenn Streiff
2008-02-21 12:39         ` Glenn Streiff
2008-02-21 15:49           ` Adrian Bunk
2008-02-21 20:28             ` Roland Dreier
2008-02-21 21:01               ` Adrian Bunk [this message]
2008-02-21 21:09                 ` Merging of completely unreviewed drivers Roland Dreier
2008-02-21 21:14                 ` Linus Torvalds
2008-02-21 22:33                   ` Alexey Dobriyan
2008-02-21 22:43                     ` Greg KH
2008-02-21 22:57                       ` Jeff Garzik
2008-02-21 22:58                       ` Alexey Dobriyan
2008-02-21 23:31                       ` Jan Engelhardt
2008-02-21 23:38                   ` Krzysztof Halasa
2008-02-21 23:31                     ` Alan Cox
2008-02-22  0:29                       ` Adrian Bunk
2008-02-21 23:41                     ` Jeff Garzik
2008-02-22  0:05                       ` Krzysztof Halasa
2008-02-22  0:44                         ` Jeff Garzik
2008-02-22  2:02                           ` Krzysztof Halasa
2008-02-22 10:04                         ` Alan Cox
2008-02-22 18:45                         ` Pavel Machek
2008-02-22 22:44                           ` Krzysztof Halasa
2008-02-23  9:43                             ` Pavel Machek
2008-02-23 12:38                               ` David Newall
2008-02-23 15:25                                 ` Pavel Machek
2008-02-24  3:18                                   ` David Newall
2008-02-23 17:33                                 ` Linus Torvalds
2008-02-24  3:26                                   ` David Newall
2008-02-24  4:47                                     ` Linus Torvalds
2008-02-23 13:58                               ` Krzysztof Halasa
2008-02-22  1:46                     ` David Newall
2008-02-22  2:06                       ` Al Viro
2008-02-22  2:23                         ` Krzysztof Halasa
2008-02-22  3:13                           ` Al Viro
2008-02-22 22:28                             ` Krzysztof Halasa
2008-02-24  7:47                               ` Jörn Engel
2008-02-24 14:47                                 ` Krzysztof Halasa
2008-02-22  3:13                         ` Linus Torvalds
2008-02-22  6:29                           ` [ofa-general] " Junio C Hamano
2008-02-22  9:02                             ` Adrian Bunk
2008-02-22  6:37                           ` Ray Lee
2008-02-23 15:31                             ` Jan Engelhardt
2008-02-24  3:22                               ` David Newall
2008-02-22 22:37                           ` Krzysztof Halasa
2008-02-22 12:29                       ` [ofa-general] " Bart Van Assche
2008-02-22 14:25                         ` David Newall
2008-02-22 15:17                           ` Peter Zijlstra
2008-02-22 16:48                             ` John W. Linville
2008-02-22 22:59                             ` Krzysztof Halasa
2008-02-22 23:14                               ` Al Viro
2008-02-22 15:48                           ` John W. Linville
2008-02-22 18:54                   ` Ingo Molnar
2008-02-22 19:11                     ` [ofa-general] " Bart Van Assche
2008-02-22 19:20                     ` Jeff Garzik
2008-02-22 19:44                       ` Greg KH
2008-02-21 21:30                 ` Greg KH
2008-02-22  1:06                   ` Adrian Bunk
2008-02-21 22:08                 ` Arjan van de Ven
2008-02-21 22:33                   ` Jeff Garzik
2008-02-21 23:40                     ` Adrian Bunk
2008-02-22 18:40                   ` Pavel Machek

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=20080221210124.GD28328@cs181133002.pp.htv.fi \
    --to=bunk@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=flatif@NetEffect.com \
    --cc=general@lists.openfabrics.org \
    --cc=greg@kroah.com \
    --cc=gstreiff@NetEffect.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rdreier@cisco.com \
    --cc=torvalds@osdl.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

all inboxes | Powered by JetHome®