Re: [PATCH 2/3] drivers/fbtft: Remove unnecessary braces from if/else

[Dan Carpenter]

On Mon, Jan 08, 2018 at 10:27:01AM -0800, Joe Perches wrote:
> On Mon, 2018-01-08 at 18:03 +0300, Dan Carpenter wrote:
> > On Mon, Jan 08, 2018 at 11:06:37AM +0100, Luis Gerhorst wrote:
> > > The Linux kernel coding style states that braces should only be used
> > > when necessary.
> > > 
> > > This fixes the checkpatch warning
> > > 
> > > WARNING: line over 80 characters
> > > +	} else if (display->regwidth == 8 && display->buswidth == 9 && par->spi) {
> > > 
> > > introduced by patch #1.
> > > 
> > 
> > Don't introduce warnings and then fix them in later patches.
> > 
> > Anyway there is another unwritten rule that multi-line indents get curly
> > braces.  Probably it should be:
> 
> Nope.  That'd be your own preferred style.
> 

I copied it from Greg so it's the subsystem style.


> If you want it to be followed by others,
> please try and get it added to CodingStyle
> and use examples to show why it's better
> than allowing maintainer preference.
> 

I'm not going to NACK patches which don't follow that rule, so it's not
something which is helpful to put in checkpatch.pl.  The reason I'm
complaining about this patch is that it fixes a patch the author
introduces earlier in the patchset.


> > 	} else if (display->regwidth == 8 && display->buswidth == 9 &&
> > 		   par->spi) {
> 
> My own preferred style here would be to align
> the display->regwidth and display->buswidth so
> I can differentiate the similarity in naming a
> bit more easily.

Yeah.  I thought about that as well but then it takes up 3 lines...
Either way is fine.

> 
> 	} else if (display->regwidth == 8 &&
> 		   display->buswidth == 9 && par->spi) {
> 
> I'm not sure at all there is a single best
> style for this and it can easily become
> situation dependent.
> 
> And so I suggest not adding anything about this
> style nit to CodingStyle.

Agreed.

regards,
dan carpenter