Moratorium on coding style patches (was Re: [PATCH] include/linux/kernel.h: fixed coding style issues)

Moratorium on coding style patches (was Re: [PATCH] include/linux/kernel.h: fixed coding style issues)

[Alexey Dobriyan]

OK, someone needs to say it.

These type of patches are advertised by some people as a good way
to enter Linux kernel development. You know, to learn how the process
works, how to setup email client pipeline, to get initial feedback.
And it is true. What those people aren't saying is that the above is
about ~0.01% of what is kernel or any other project development is about.
It is the easy part.

But the patches also create problems for those who are already in.
The very immediate is that "git-blame" stops working. It simply points
to the irrelevant commit and developer is forced to either search
manually through the history or search the web for "git-blame" options.
(maybe there is such an option but that's not the point).

And "git-blame" usually happens in important cases: when developer
searches for a possible bugfix or wondering who wrote that crap.


Au contraire, coding style patch is something unimportant.
Whitespace here, whitespace there, who cares. On the grand scale,
coding style compliance is important but in my experience Linux
kernel CS compliance is top notch for the project of Linux's size.

So the tradeoff is not in the patch favour and all you need to follow
coding style is basically "indent -kr -i8 -l80" for new code.
It is then becomes non problem because editors defaults are doing
the job.

And they create rejects against other non-coding style patches,
again slowing down people who need to fix real problems.

Then there is whole big "newbie" angle.

My first patches were sent to kernel-janitors@ list which was setup
exactly for newbies. Then I agreed to become its maintainer and then
(what a shame) silently killed it by doing nothing. But it was not
a concious decision. Frankly I do not remember _exactly_ what happened.

Then SWsoft hired me to do kernel work (hi, Kirill and Pavel! and Den!)
In the first month I learned more about kernel internals than in
the previous years of self tinkering. Hey, I didn't know about ctags
(or never botherd to find out). More importantly I learned which bugs
and problems actually happen to users (or rather customers, commercial
Linux doesn't have users, only customers :-) and which exist only in
my head.

So my advise to Piotr. I can't find your name or email in the changelogs.
It looks like this is your first patch, my apologies if it is not.

Find and fix a bug.

It may be hard to find a bug for a newcomer, there is relaxed version:
if your patch doesn't change generated code, then don't send it.

Simply don't do it.

You'll send them but later when you learn how the kernel works and
what is important and what is not.

Kernel will not collapse because whitespace is a bit off.
But it will collapse if people would only fix coding style.

So you need to grow as a kernel developer, and the sooner you start,
the sooner you'll be there.

----------------------------------------------------------------------

Said that, I call for a tree wide moratorium on pure coding style changes.

Re: Moratorium on coding style patches (was Re: [PATCH] include/linux/kernel.h: fixed coding style issues)

[Måns Rullgård]

Alexey Dobriyan <adobriyan@gmail.com> writes:

> OK, someone needs to say it.
>
> These type of patches are advertised by some people as a good way
> to enter Linux kernel development. You know, to learn how the process
> works, how to setup email client pipeline, to get initial feedback.
> And it is true. What those people aren't saying is that the above is
> about ~0.01% of what is kernel or any other project development is about.
> It is the easy part.
>
> But the patches also create problems for those who are already in.
> The very immediate is that "git-blame" stops working. It simply points
> to the irrelevant commit and developer is forced to either search
> manually through the history or search the web for "git-blame" options.
> (maybe there is such an option but that's not the point).
>
> And "git-blame" usually happens in important cases: when developer
> searches for a possible bugfix or wondering who wrote that crap.
>
> Au contraire, coding style patch is something unimportant.
> Whitespace here, whitespace there, who cares. On the grand scale,
> coding style compliance is important but in my experience Linux
> kernel CS compliance is top notch for the project of Linux's size.
>
> So the tradeoff is not in the patch favour and all you need to follow
> coding style is basically "indent -kr -i8 -l80" for new code.
> It is then becomes non problem because editors defaults are doing
> the job.
>
> And they create rejects against other non-coding style patches,
> again slowing down people who need to fix real problems.

I agree with all of that, yet I still sometimes create pure style
patches.  This tends to happen when I can't look at the code I'm trying
to debug without wanting to claw my eyes out.  In such cases, an initial
cleanup patch often makes the actual fix much easier to comprehend.
There's not a lot such code in the kernel, thankfully, but it does
exist.

> Said that, I call for a tree wide moratorium on pure coding style changes.

In light of the above, I'd relax this slightly and say don't do style
patches unless as a prelude to some real work.

-- 
Måns Rullgård

Re: Moratorium on coding style patches (was Re: [PATCH] include/linux/kernel.h: fixed coding style issues)

[Joe Perches]

On Wed, 2016-12-14 at 18:13 +0000, Måns Rullgård wrote:
> Alexey Dobriyan <adobriyan@gmail.com> writes:
> > I call for a tree wide moratorium on pure coding style changes.
[]
> I'd relax this slightly and say don't do style
> patches unless as a prelude to some real work.

And exclude drivers/staging

Re: Moratorium on coding style patches (was Re: [PATCH] include/linux/kernel.h: fixed coding style issues)

[Måns Rullgård]

Joe Perches <joe@perches.com> writes:

> On Wed, 2016-12-14 at 18:13 +0000, Måns Rullgård wrote:
>> Alexey Dobriyan <adobriyan@gmail.com> writes:
>> > I call for a tree wide moratorium on pure coding style changes.
> []
>> I'd relax this slightly and say don't do style
>> patches unless as a prelude to some real work.
>
> And exclude drivers/staging

I wouldn't mind seeing staging deleted.  What makes those drivers so
special that they deserve to be included in the main tree without
meeting even the most basic of standards normally required?

-- 
Måns Rullgård

Re: Moratorium on coding style patches (was Re: [PATCH] include/linux/kernel.h: fixed coding style issues)

[Joe Perches]

(Adding Greg KH)

On Thu, 2016-12-15 at 11:54 +0000, Måns Rullgård wrote:
> Joe Perches <joe@perches.com> writes:
> 
> > On Wed, 2016-12-14 at 18:13 +0000, Måns Rullgård wrote:
> > > Alexey Dobriyan <adobriyan@gmail.com> writes:
> > > > I call for a tree wide moratorium on pure coding style changes.
> > 
> > []
> > > I'd relax this slightly and say don't do style
> > > patches unless as a prelude to some real work.
> > 
> > And exclude drivers/staging
> 
> I wouldn't mind seeing staging deleted.  What makes those drivers so
> special that they deserve to be included in the main tree without
> meeting even the most basic of standards normally required?

People actually use them. (well, some of them)

Re: Moratorium on coding style patches (was Re: [PATCH] include/linux/kernel.h: fixed coding style issues)

[Greg KH]

On Thu, Dec 15, 2016 at 11:54:54AM +0000, Måns Rullgård wrote:
> Joe Perches <joe@perches.com> writes:
> 
> > On Wed, 2016-12-14 at 18:13 +0000, Måns Rullgård wrote:
> >> Alexey Dobriyan <adobriyan@gmail.com> writes:
> >> > I call for a tree wide moratorium on pure coding style changes.
> > []
> >> I'd relax this slightly and say don't do style
> >> patches unless as a prelude to some real work.
> >
> > And exclude drivers/staging
> 
> I wouldn't mind seeing staging deleted.  What makes those drivers so
> special that they deserve to be included in the main tree without
> meeting even the most basic of standards normally required?

Because it provides both a place where people can learn in a "safe"
place, and it allows companies to get their code into the tree so that
others can use it, and it allows users to use their hardware when there
is no other driver availble and no one has done the work to get it
cleaned up yet "properly".

If you don't like staging, fine, ignore it, but don't ask to have it be
deleted if you don't even understand why it was created, why it is used,
and who it is for.

Basic education and manners people, please.

greg k-h