Re: [PATCH] scsi: megaraid: cleanup formatting of megaraid

From: Finn Thain
Date: Thu Mar 03 2022 - 17:44:27 EST


Hi Konrad,

On Thu, 3 Mar 2022, Konrad Wilhelm Kleine wrote:

> On Thu, 3 Mar 2022 at 09:40, Finn Thain <fthain@xxxxxxxxxxxxxx> wrote:
>
> > On Wed, 2 Mar 2022, Tom Rix wrote:
> >
> > > >>> Long term, it would be good have a reliable way to automatically
> > > >>> fix either new files or really broken old files.
> > > >> That's really a maintainer preference no?
> > > >>
> > > >> Especially so for any automation.
> > > >
> > > > In practice everything is up to the maintainer.
> > > >
> > > > If some maintainer wants fix their formatting then clang-format
> > > > should just work
> > > >
> > > > It isn't likely they will have time to hand fix every file.
> > >
> > > A follow up issue in the clang project has been raised by Konrad,
> > > here
> > >
> > > https://github.com/llvm/llvm-project/issues/54137
> > >
> >
> > Why request a "leave" option for every style rule? Why not just a
> > "leave" option for the most contentious rules?
> >
>
> Getting to the point that every style option can be disabled
> individually is not an operation done in one go. I plan on presenting
> the changes required to exactly one style option and from there I'm all
> ears if you have style options that you consider "contentious".

Sure, I'll provide an example of that below in case you're interested in
what I happen to find contentious. But this isn't about my taste.

The issue is really about removing an obstacle to wider adoption of
clang-formt. Therefore, shouldn't you be asking, what style rules have
already proven to be contentious within the project?

You can look to kernel subsystem style rules for examples of that. E.g.
some might argue that comments should always be left unmolested (where
they exist for the benefit of human readers and not tooling).

Others might argue that they should always be changed from,

/*
* this style
* of multiline comment
*/

to

/* this style
* of multiline comment
*/

For another example, the mailing lists recently saw another style rule
difference between subsystems:
https://lore.kernel.org/all/20220301055231.GI2812@kadam/

Joe said, in effect, "leave" whereas others seem to have different views.

Finally, here's an example that I personally found contentious.
drivers/scsi/NCR5380.c line 2306:
dsprintk(NDEBUG_ABORT, instance,
"abort: removed %p from sense queue\n", cmd);

Note the spaces (for alignment) following the tabs (for scope). I mention
this example not because I expect the world to "wake up and smell the
coffee" one day and embrace spaces-after-tabs.

I mention it because I expect the tabs/spaces/both issue to remain
contentious indefinitely (within any sufficiently large project).

I can think of another good example (line wrap) but I'll stop here.