* Re: switch identation was: Re: [Kernel-janitors] [PATCH] Fix
@ 2004-01-08 0:45 Randy.Dunlap
2004-01-08 7:36 ` Luiz Fernando Capitulino
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: Randy.Dunlap @ 2004-01-08 0:45 UTC (permalink / raw)
To: kernel-janitors
On 24 Dec 2003 01:44:25 -0000 "Luiz Capitulino" <lcapitulino@terra.com.br> wrote:
|
| On 12/24/2003, "Arnaldo Carvalho de Melo" <acme@conectiva.com.br> wrote:
|
| >Em Wed, Dec 24, 2003 at 01:29:15AM -0000, Luiz Capitulino escreveu:
| >>
| >> Hi Arnaldo,
| >>
| >> On 12/23/2003, "Arnaldo Carvalho de Melo" <acme@conectiva.com.br> wrote:
| >>
| >> >Em Tue, Dec 23, 2003 at 10:43:16AM -0200, Luiz Fernando Capitulino escreveu:
| >> >> + switch(clocking) {
| >> >> + case 0x03: printk("DISABLED !\n"); break;
| >> >> + case 0x02: printk("= 2X PCI \n"); break;
| >> >> + case 0x01: printk("= 133 \n"); break;
| >> >> + case 0x00: printk("= 100 \n"); break;
| >> >> + }
| >> >
| >> >May I suggest that we use:
| >> >
| >> > switch(clocking) {
| >> > case 0x03: printk("DISABLED !\n"); break;
| >> > case 0x02: printk("= 2X PCI \n"); break;
| >> > case 0x01: printk("= 133 \n"); break;
| >> > case 0x00: printk("= 100 \n"); break;
| >> > }
| >> >
| >> >That is how it is done in the networking code and most of the core (fs/,
| >> >kernel/) I've seen.
| >>
| >> I can rediff, but the problem is that all the switch()s in siimage.c
| >> have the indention which I did.
| >>
| >> Should I rediff ?
No. But that
+ if(! pdev_is_sata(dev)) {
is really ugly (the spacing in "if(! " part of it).
And all this warning needs to fix it is an additional semi-colon, like:
-sata_skip:
+sata_skip:;
although I don't disapprove of the intent of your patch.
| >Ah, indeed having different styles in the same file is something awkward...
| >Perhaps you rediff and later submit a coding style patch converting the other
| >switches?
|
| I think is better let this patch with the some style of the file, and
| later I do a new patch which change all the switch()s (is what you
| said? :-))
|
| Should it be a new janitor task ?
|
| Add this rule to CodingStyle should be good too.
I prefer to use switch() formatting as acme has suggested, but
changing it in source files that are currently maintained should go
thru their maintainer(s) IMO.
For source files that are basically unmaintained, I don't mind
pushing formatting changes.
--
~Randy
MOTD: Always include version info.
--
~Randy
MOTD: Always include version info.
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: switch identation was: Re: [Kernel-janitors] [PATCH] Fix
2004-01-08 0:45 switch identation was: Re: [Kernel-janitors] [PATCH] Fix Randy.Dunlap
@ 2004-01-08 7:36 ` Luiz Fernando Capitulino
2004-01-08 19:40 ` Randy.Dunlap
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Luiz Fernando Capitulino @ 2004-01-08 7:36 UTC (permalink / raw)
To: kernel-janitors
Hi Randy,
Em Qua, 2004-01-07 às 22:45, Randy.Dunlap escreveu:
> | >> Should I rediff ?
>
> No. But that
> + if(! pdev_is_sata(dev)) {
> is really ugly (the spacing in "if(! " part of it).
>
> And all this warning needs to fix it is an additional semi-colon, like:
>
> -sata_skip:
> +sata_skip:;
>
> although I don't disapprove of the intent of your patch.
So, I will rediff only for the "if(! ". Ok ?
> I prefer to use switch() formatting as acme has suggested, but
> changing it in source files that are currently maintained should go
> thru their maintainer(s) IMO.
>
> For source files that are basically unmaintained, I don't mind
> pushing formatting changes.
Should it go to TODO file ?
--
Luiz Fernando N. Capitulino
<lcapitulino@prefeitura.sp.gov.br>
<http://www.telecentros.sp.gov.br>
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: switch identation was: Re: [Kernel-janitors] [PATCH] Fix
2004-01-08 0:45 switch identation was: Re: [Kernel-janitors] [PATCH] Fix Randy.Dunlap
2004-01-08 7:36 ` Luiz Fernando Capitulino
@ 2004-01-08 19:40 ` Randy.Dunlap
2004-01-08 20:13 ` switch identation was: Re: [Kernel-janitors] [PATCH] Fix warning Arnaldo Carvalho de Melo
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Randy.Dunlap @ 2004-01-08 19:40 UTC (permalink / raw)
To: kernel-janitors
On Thu, 08 Jan 2004 05:36:20 -0200 Luiz Fernando Capitulino <lcapitulino@prefeitura.sp.gov.br> wrote:
|
| Hi Randy,
|
| Em Qua, 2004-01-07 às 22:45, Randy.Dunlap escreveu:
|
| > | >> Should I rediff ?
| >
| > No. But that
| > + if(! pdev_is_sata(dev)) {
| > is really ugly (the spacing in "if(! " part of it).
| >
| > And all this warning needs to fix it is an additional semi-colon, like:
| >
| > -sata_skip:
| > +sata_skip:;
| >
| > although I don't disapprove of the intent of your patch.
|
| So, I will rediff only for the "if(! ". Ok ?
OK, I'll merge that, except that if and switch are not functions,
so we usually put a space after them and before the opening '('
[yes, I'm slightly disagreeing with acme's proposed switch formatting].
|
| > I prefer to use switch() formatting as acme has suggested, but
| > changing it in source files that are currently maintained should go
| > thru their maintainer(s) IMO.
| >
| > For source files that are basically unmaintained, I don't mind
| > pushing formatting changes.
|
| Should it go to TODO file ?
Sure, I'll add that.
--
~Randy
--
~Randy
MOTD: Always include version info.
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: switch identation was: Re: [Kernel-janitors] [PATCH] Fix warning
2004-01-08 0:45 switch identation was: Re: [Kernel-janitors] [PATCH] Fix Randy.Dunlap
2004-01-08 7:36 ` Luiz Fernando Capitulino
2004-01-08 19:40 ` Randy.Dunlap
@ 2004-01-08 20:13 ` Arnaldo Carvalho de Melo
2004-01-08 20:19 ` Arnaldo Carvalho de Melo
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2004-01-08 20:13 UTC (permalink / raw)
To: kernel-janitors
Em Thu, Jan 08, 2004 at 11:40:04AM -0800, Randy.Dunlap escreveu:
> On Thu, 08 Jan 2004 05:36:20 -0200 Luiz Fernando Capitulino <lcapitulino@prefeitura.sp.gov.br> wrote:
>
> |
> | Hi Randy,
> |
> | Em Qua, 2004-01-07 às 22:45, Randy.Dunlap escreveu:
> |
> | > | >> Should I rediff ?
> | >
> | > No. But that
> | > + if(! pdev_is_sata(dev)) {
> | > is really ugly (the spacing in "if(! " part of it).
> | >
> | > And all this warning needs to fix it is an additional semi-colon, like:
> | >
> | > -sata_skip:
> | > +sata_skip:;
> | >
> | > although I don't disapprove of the intent of your patch.
> |
> | So, I will rediff only for the "if(! ". Ok ?
>
> OK, I'll merge that, except that if and switch are not functions,
> so we usually put a space after them and before the opening '('
> [yes, I'm slightly disagreeing with acme's proposed switch formatting].
not at all, if I proposed something like:
switch(foo) {
.
.
.
I'd be nuts 8)
switch (foo) {
case bar:
__bar();
break;
case baz:
__baz();
break;
default:
printk(KERN_ERR "whatever\n");
break;
}
Is how I write my code and fix other's :)
- Arnaldo
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: switch identation was: Re: [Kernel-janitors] [PATCH] Fix warning
2004-01-08 0:45 switch identation was: Re: [Kernel-janitors] [PATCH] Fix Randy.Dunlap
` (2 preceding siblings ...)
2004-01-08 20:13 ` switch identation was: Re: [Kernel-janitors] [PATCH] Fix warning Arnaldo Carvalho de Melo
@ 2004-01-08 20:19 ` Arnaldo Carvalho de Melo
2004-01-09 12:26 ` switch identation was: Re: [Kernel-janitors] [PATCH] Fix Luiz Fernando Capitulino
2004-01-12 3:32 ` Randy.Dunlap
5 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2004-01-08 20:19 UTC (permalink / raw)
To: kernel-janitors
Em Tue, Dec 23, 2003 at 03:18:50PM -0200, Arnaldo C. Melo escreveu:
> Em Tue, Dec 23, 2003 at 10:43:16AM -0200, Luiz Fernando Capitulino escreveu:
> > + switch(clocking) {
> > + case 0x03: printk("DISABLED !\n"); break;
> > + case 0x02: printk("= 2X PCI \n"); break;
> > + case 0x01: printk("= 133 \n"); break;
> > + case 0x00: printk("= 100 \n"); break;
> > + }
>
> May I suggest that we use:
>
> switch(clocking) {
I was nuts ;(
- Arnaldo
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: switch identation was: Re: [Kernel-janitors] [PATCH] Fix
2004-01-08 0:45 switch identation was: Re: [Kernel-janitors] [PATCH] Fix Randy.Dunlap
` (3 preceding siblings ...)
2004-01-08 20:19 ` Arnaldo Carvalho de Melo
@ 2004-01-09 12:26 ` Luiz Fernando Capitulino
2004-01-12 3:32 ` Randy.Dunlap
5 siblings, 0 replies; 7+ messages in thread
From: Luiz Fernando Capitulino @ 2004-01-09 12:26 UTC (permalink / raw)
To: kernel-janitors
Hi Randy,
Em Qui, 2004-01-08 às 17:40, Randy.Dunlap escreveu:
> OK, I'll merge that, except that if and switch are not functions,
> so we usually put a space after them and before the opening '('
Oh yes, I use that too (I think this way comes from K&R), but in this
patch I'm following the function formatting (as you can see in the lines
which was removed).
Actually, Alan (I think him wrote the code) are using the two ways
in the siimage.c code!
--
Luiz Fernando N. Capitulino
<lcapitulino@prefeitura.sp.gov.br>
<http://www.telecentros.sp.gov.br>
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: switch identation was: Re: [Kernel-janitors] [PATCH] Fix
2004-01-08 0:45 switch identation was: Re: [Kernel-janitors] [PATCH] Fix Randy.Dunlap
` (4 preceding siblings ...)
2004-01-09 12:26 ` switch identation was: Re: [Kernel-janitors] [PATCH] Fix Luiz Fernando Capitulino
@ 2004-01-12 3:32 ` Randy.Dunlap
5 siblings, 0 replies; 7+ messages in thread
From: Randy.Dunlap @ 2004-01-12 3:32 UTC (permalink / raw)
To: kernel-janitors
On Thu, 08 Jan 2004 05:36:20 -0200 Luiz Fernando Capitulino <lcapitulino@prefeitura.sp.gov.br> wrote:
|
| > I prefer to use switch() formatting as acme has suggested, but
| > changing it in source files that are currently maintained should go
| > thru their maintainer(s) IMO.
| >
| > For source files that are basically unmaintained, I don't mind
| > pushing formatting changes.
|
| Should it go to TODO file ?
Added to KJ TODO file.
--
~Randy
MOTD: Always include version info.
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2004-01-12 3:32 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-01-08 0:45 switch identation was: Re: [Kernel-janitors] [PATCH] Fix Randy.Dunlap
2004-01-08 7:36 ` Luiz Fernando Capitulino
2004-01-08 19:40 ` Randy.Dunlap
2004-01-08 20:13 ` switch identation was: Re: [Kernel-janitors] [PATCH] Fix warning Arnaldo Carvalho de Melo
2004-01-08 20:19 ` Arnaldo Carvalho de Melo
2004-01-09 12:26 ` switch identation was: Re: [Kernel-janitors] [PATCH] Fix Luiz Fernando Capitulino
2004-01-12 3:32 ` Randy.Dunlap
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.