All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andi Kleen <ak@suse.de>
To: Sam Ravnborg <sam@ravnborg.org>
Cc: Andi Kleen <ak@suse.de>, Ingo Molnar <mingo@elte.hu>,
	peterz@infradead.org, linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH x86] [5/16] Replace hard coded reservations in x86-64 early boot code with dynamic table II
Date: Fri, 4 Jan 2008 19:06:50 +0100	[thread overview]
Message-ID: <20080104180650.GA2998@bingen.suse.de> (raw)
In-Reply-To: <20080104175128.GB31382@uranus.ravnborg.org>

> What is then considered "obvious style issues" are today implemented
> as errors / warnings in checkpatch for the most part. Not everyone

In my experience checkpatch has a lot of false positives. e.g. when 
I run it over my pile 20+% of the warnings are just bogus. And in
a lot of other cases it is that the rules of thumbs it applies
do not apply to that specific case. 

e.g. example: when you have self test debugging code it is perfectly
fine to not use KERN_*

> agrees with what checkpatch says are errors/warnings but this is at
> least an more or less unambigious way to express these obvious cases.

It's not -- checkpatch is often wrong. 

> And this is far better than the maintainers choice of the day style.

You mean the one from Ingo with the random new lines? @) Perhaps.
I just ask that people use some common sense.

> > Same for tabs versus spaces. That is something no human should
> > need to care about. And it is something a machine can handle fine too 
> > anyways.
> Things turn red in my vi if no-one cares 

That's a problem of your vi. I'm sure it can be turned off.

Fortunately it's easily fixed.

>- and I have no magic tool
> to fix it here.

I wrote one now.

ftp://ftp.firstfloor.org/pub/ak/perl/converttabs

As you can see it was trivial.

> No-one should submit patches with white-space errors - thank you.

No I disagree. If white space is suddenly that important -- it is still
unclear to me why that should be -- then the right way to handle that is 
at the maintainer level while merging patches. But rejecting patches
for reasons that a trivial script can fix up is just unnecessary friction
in the merging process and unnecessary bureaucracy.

Unnecessary bureaucracy defined as in rituals that do not 
actually improve any code or prevent bugs (or even style). I must say I find 
it a little sad that Linux is dissolving into that now. Ok perhaps
it is the eventual fate of anything that has been around for quite a long time,
but it seems to get worse much quicker recently.

Anyways I have the script above included now here in my patch setup
scripts so future patches of mine will have tabs.

But the problem with your "rule" is that every of the hundreds
of Linux kernel contributors will need to do the same. And that's just 
ineffecient, waste of valuable human work and just doesn't make much sense.

It certainly won't improve Linux in any way.

-Andi


  reply	other threads:[~2008-01-04 18:07 UTC|newest]

Thread overview: 103+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-03 15:42 [PATCH x86] [0/16] Various i386/x86-64 changes Andi Kleen
2008-01-03 15:42 ` [PATCH x86] [1/16] Make clocksource watchdog cycle through online CPUs Andi Kleen
2008-01-04  8:51   ` Ingo Molnar
2008-01-04 12:59     ` Andi Kleen
2008-01-03 15:42 ` [PATCH x86] [2/16] Add a counter for per cpu clocksource watchdog checks and report them in /proc/interrupts Andi Kleen
2008-01-04  8:53   ` Ingo Molnar
2008-01-03 15:42 ` [PATCH x86] [3/16] Turn irq debugging options into module params Andi Kleen
2008-01-04  8:55   ` Ingo Molnar
2008-01-03 15:42 ` [PATCH x86] [4/16] Add /proc/irq/*/spurious to dump the spurious irq debugging state Andi Kleen
2008-01-04  8:58   ` Ingo Molnar
2008-01-04 12:22     ` Andi Kleen
2008-01-03 15:42 ` [PATCH x86] [5/16] Replace hard coded reservations in x86-64 early boot code with dynamic table Andi Kleen
2008-01-04  9:00   ` Ingo Molnar
2008-01-04 12:23     ` Andi Kleen
2008-01-04 12:38     ` [PATCH x86] [5/16] Replace hard coded reservations in x86-64 early boot code with dynamic table II Andi Kleen
2008-01-04 14:41       ` Ingo Molnar
2008-01-04 15:03         ` Andi Kleen
2008-01-04 17:51           ` Sam Ravnborg
2008-01-04 18:06             ` Andi Kleen [this message]
2008-01-04 18:47               ` Joe Perches
2008-01-04 22:13                 ` Andi Kleen
2008-01-04 22:46                   ` J. Bruce Fields
2008-01-04 20:56               ` Sam Ravnborg
2008-01-04 12:41   ` [PATCH x86] [5/16] Replace hard coded reservations in x86-64 early boot code with dynamic table Cyrill Gorcunov
2008-01-03 15:42 ` [PATCH x86] [6/16] Add a new arch_early_alloc() interface for x86-64 Andi Kleen
2008-01-03 16:22   ` Eric Dumazet
2008-01-03 17:17     ` Andi Kleen
2008-01-03 15:42 ` [PATCH x86] [7/16] Convert lockdep to use arch_early_alloc() if available for its large arrays Andi Kleen
2008-01-04  9:03   ` Ingo Molnar
2008-01-03 15:42 ` [PATCH x86] [8/16] Make lockdep_init __init Andi Kleen
2008-01-04  9:06   ` Ingo Molnar
2008-01-03 15:42 ` [PATCH x86] [9/16] Remove CPU capabitilites printks on i386 Andi Kleen
2008-01-04  9:11   ` Ingo Molnar
2008-01-03 15:42 ` [PATCH x86] [10/16] Document fdimage/isoimage completely in make help Andi Kleen
2008-01-04  9:00   ` Sam Ravnborg
2008-01-04  9:15     ` Ingo Molnar
2008-01-03 15:42 ` [PATCH x86] [11/16] Compile apm and voyager module only when selected in Kconfig Andi Kleen
2008-01-04  4:15   ` Stephen Rothwell
2008-01-03 15:42 ` [PATCH x86] [12/16] Optimize lock prefix switching to run less frequently Andi Kleen
2008-01-04  9:42   ` Thomas Gleixner
2008-01-04 12:17     ` Andi Kleen
2008-01-04 14:19       ` Thomas Gleixner
2008-01-04 14:39         ` Andi Kleen
2008-01-04 15:02           ` Pekka Enberg
2008-01-04 15:04             ` Andi Kleen
2008-01-04 20:34           ` Thomas Gleixner
2008-01-04 22:11             ` Andi Kleen
2008-01-05  1:19               ` Ingo Molnar
2008-01-05 14:37                 ` Andi Kleen
2008-01-03 15:42 ` [PATCH x86] [13/16] i386: Set CFQ as default in i386 defconfig Andi Kleen
2008-01-04  9:19   ` Ingo Molnar
2008-01-03 15:42 ` [PATCH x86] [14/16] Enable RDC321X subarch only on 32bit Andi Kleen
2008-01-04  9:22   ` Ingo Molnar
2008-01-03 15:42 ` [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU Andi Kleen
2008-01-03 17:22   ` Rafael J. Wysocki
2008-01-03 17:42     ` Andi Kleen
2008-01-03 21:41       ` Rafael J. Wysocki
2008-01-04  9:23         ` Ingo Molnar
2008-01-10 17:14           ` Rafael J. Wysocki
2008-01-03 18:14   ` Adrian Bunk
2008-01-03 18:43     ` Andi Kleen
2008-01-10  9:54       ` Adrian Bunk
2008-01-10  9:58         ` Andi Kleen
2008-01-10 10:19           ` Adrian Bunk
2008-01-10 11:15             ` Andi Kleen
2008-01-10 11:26               ` Adrian Bunk
2008-01-10 11:42                 ` Andi Kleen
2008-01-10 12:47                   ` Adrian Bunk
2008-01-10 13:12                     ` Andi Kleen
2008-01-10 15:09                       ` Adrian Bunk
2008-01-14 13:52                         ` Ingo Molnar
2008-01-14 14:09                           ` Sam Ravnborg
2008-01-14 14:58                             ` Ingo Molnar
2008-01-14 15:01                               ` Ingo Molnar
2008-01-14 18:20                                 ` Adrian Bunk
2008-01-14 19:10                                   ` Ingo Molnar
2008-01-14 19:52                                     ` Adrian Bunk
2008-01-14 20:01                                       ` Sam Ravnborg
2008-01-14 20:18                                         ` Adrian Bunk
2008-01-14 20:27                                           ` Sam Ravnborg
2008-01-14 20:34                                             ` Adrian Bunk
2008-01-15 22:14                                           ` Ingo Molnar
2008-01-15 22:51                                             ` Adrian Bunk
2008-01-14 19:59                                 ` Sam Ravnborg
2008-01-15 22:12                                   ` [patch for v2.6.24] fix section mismatch warning in page_alloc.c Ingo Molnar
2008-01-14 15:05                               ` [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU Ingo Molnar
2008-01-14 15:24                                 ` Ingo Molnar
2008-01-14 20:12                                   ` Sam Ravnborg
2008-01-15 15:17                                     ` Ingo Molnar
2008-01-15 16:25                                       ` Sam Ravnborg
2008-01-15 17:11                                         ` Andi Kleen
2008-01-15 17:27                                           ` Adrian Bunk
2008-01-15 18:21                                           ` Sam Ravnborg
2008-01-15 18:29                                             ` Andi Kleen
2008-01-15 18:31                                               ` Sam Ravnborg
2008-01-15 18:47                                             ` Adrian Bunk
2008-01-14 18:17                               ` Adrian Bunk
2008-01-14 15:25                           ` Adrian Bunk
2008-01-10 17:17                   ` Rafael J. Wysocki
2008-01-11 23:06                     ` [PATCH] x86: Change unnecessary dependencies on CONFIG_PM Rafael J. Wysocki
2008-01-03 15:42 ` [PATCH x86] [16/16] Mark memory_setup __init Andi Kleen
2008-01-04  9:25   ` Ingo Molnar
2008-01-04  9:53     ` Ingo Molnar

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=20080104180650.GA2998@bingen.suse.de \
    --to=ak@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=sam@ravnborg.org \
    --cc=tglx@linutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.