Re: switch identation was: Re: [Kernel-janitors] [PATCH] Fix

Re: switch identation was: Re: [Kernel-janitors] [PATCH] Fix

[Randy.Dunlap]

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

Re: switch identation was: Re: [Kernel-janitors] [PATCH] Fix

[Luiz Fernando Capitulino]

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

Re: switch identation was: Re: [Kernel-janitors] [PATCH] Fix

[Randy.Dunlap]

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

Re: switch identation was: Re: [Kernel-janitors] [PATCH] Fix warning

[Arnaldo Carvalho de Melo]

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

Re: switch identation was: Re: [Kernel-janitors] [PATCH] Fix warning

[Arnaldo Carvalho de Melo]

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

Re: switch identation was: Re: [Kernel-janitors] [PATCH] Fix

[Luiz Fernando Capitulino]

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

Re: switch identation was: Re: [Kernel-janitors] [PATCH] Fix

[Randy.Dunlap]

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