Re: [Ksummit-discuss] [MAINTAINERS SUMMIT] Pull network and Patch Acceptance Consistency

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Thu, 2019-06-13 at 14:27 -0300, Mauro Carvalho Chehab wrote:
> Em Thu, 13 Jun 2019 07:35:07 -0700
> James Bottomley <James.Bottomley@HansenPartnership.com> escreveu:
> > It depends: every patch you do to an old driver comes with a risk
> > of breakage.  What we've found is even apparently sane patches
> > cause breakage which isn't discovered until months later when
> > someone with the hardware actually tests. 
> 
> True, but, if you do the diff between the .o file produced before
> the patch and after it (and/or the associated .a file), you should be
> able to discover if the change caused a regression or not.
> 
> So, if the patch is a "pure" coding style fix, you could be able to
> avoid regressions.

Right, that's why I said "doesn't change the binary in the rules lower
down".  However, the number of people who actually come with a same
binary before and after section in their changelog is tiny ...

So perhaps we should document somewhere how (or even provide a tool) to
demonstrate the binary remains the same across the patch, because it is
an enormous help to subsystem maintainers.

> > So the general rule is:
> > 
> >    1. No whitespace/style changes to old drivers without a fix as
> > well
> 
> Yeah, we don't allow that either (except on staging - and on special
> cases).
> 
> When I started as media maintainer, I did some whitespace/tabs/indent
> cleaning, as it is easier to maintain a clean house.

We did this for some drivers, but usually only when changing
maintainers.  Even if a maintainer has slightly esoteric style ideas,
it's usually better to keep them happy than to be pedantic about
enforcing kernel style.

James

> >    2. We might take changes in comments only (spelling updates or
> > licence
> >       stuff) and other stuff that provably doesn't alter the
> > binary.
> >    3. Fixes which are tested on the actual hardware are welcome.
> >    4. Any "obvious" bug fixes which aren't hardware tested really
> > have to
> >       be obvious and well inspected (these are the ones that
> > usually cause
> >       the problems)
> >    5. Systemwide sweeps we do and usually just pray it was right
> > 
> > However, if someone comes along with the actual hardware to test
> > and wants to take over maintaining it, they pretty much get carte
> > blance to do whatever they want (see NCR 5380), so the above only
> > applies to unmaintained old drivers.
> > 
> > > In the case of media we have 20+ years of changes. So, the
> > > received code had a myriad of different coding styles.
> > > 
> > > Yet, we do enforce the current coding practices to all new code
> > > we receive.  
> > 
> > We don't.  We enforce style in the existing driver for readability
> > and consistency unless you're the maintainer of the driver and wish
> > to change it.  Then we'd insist on changing it to kernel style.
> > 
> > > Also, at least at the core (and on some drivers that people use
> > > as  reference for new codes), when we receive patches that do a
> > > large  amount of changes at the code, and we have some spare
> > > time, we run checkpatch.pl at the entire affected file, and we
> > > fix the style as much as possible[1].  
> > 
> > We have done this, but only if the Maintainer wants to do it.  For
> > drivers with no maintainer, we definitely don't.
> > 
> > James
> > 
> > > Yeah, that's painful, but as we do such practices for quite sime
> > > time, nowadays the code gets improved and now people tend to do
> > > first time  submissions using the current style practices.
> > > 
> > > [1] We usually ignore 80 column warnings on legacy code though,
> > > as a proper fix would mean to rewrite the code in order to split
> > > functions into smaller ones, with could cause regressions.
> > > 
> > > Thanks,
> > > Mauro
> > > _______________________________________________
> > > Ksummit-discuss mailing list
> > > Ksummit-discuss@lists.linuxfoundation.org
> > > https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discus
> > > s
> > >   
> 
> 
> 
> Thanks,
> Mauro
> _______________________________________________
> Ksummit-discuss mailing list
> Ksummit-discuss@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss
>