Whitespace and bullying

Andrew Bartlett abartlet at samba.org
Sun Apr 8 22:41:07 UTC 2018


On Sat, 2018-04-07 at 20:32 +1000, Martin Schwenke wrote:
> On Sat, 07 Apr 2018 20:18:29 +1200, Andrew Bartlett
> <abartlet at samba.org> wrote:
> 
> > On Sat, 2018-04-07 at 16:44 +1000, Martin Schwenke wrote:
> > > One problem is that if a subsystem has a lot of failures (perhaps even
> > > just trailing whitespace in comments?), then the flag will be off for a
> > > long time (forever?) and we won't catch new problems.  
> > We need to practice what we preach.  Many contributors (myself if you
> > need a specific example) pattern far better by example of the code
> > nearby than rule-books. 
> > If the majority of our code doesn't comply with README.Coding then the
> > rule shouldn't be in README.Coding.  
> 
> While I agree, I'm working hard to accomplish some particular goals.
> Those goals don't include getting sidetracked by having to fix
> coding standard violations in comments in code that I'm hoping to
> remove.  I already feel over-committed and under pressure.

I totally agree, but in a different way.  

The sheer amount of time and passion directed on this point, is why I'm
calling bullshit on this rule, and why I say this isn't really about
whitespace at all, but about power and control.

Asking developers, new or experienced, to match a whitespace guide that
you can't see without filters and that much of our code doesn't honour
on pain of public flagellation (but without our CI advising ahead of
time) is just stupid.

We could perhaps save that for code going in without tests, which
happens all the time and isn't commented on, or actually saying well
done, like Andreas did.

If the whole team held to that positive review culture, or if those who
continue to raise had made any effort to read the actual code to
provide a balanced an productive comment, that would be a very
different thing.  But code review by a significant number of team
members, Volker in particular, has degenerated into the two categories
of:
 - 80 Columns
and
 - whitespace

Now, I've just make it personal above, and perhaps this is where this
thread will totally derail.  

I'm also sure that changing this rule won't change the review culture,
but if even a sliver of the passion is translated into actually
checking the code, that would be a good thing.

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT   
https://catalyst.net.nz/services/samba







More information about the samba-technical mailing list