Whitespace and bullying

[Swen Schillig]

On Mon, 2018-04-09 at 21:11 +1000, Martin Schwenke via samba-technical
wrote:
> On Mon, 9 Apr 2018 09:56:00 +1000, ronnie sahlberg
> <ronniesahlberg at gmail.com> wrote:
> 
> > Not sure I understand why this topic is so controversial and needs
> > so
> > much discussion.
> > 
> > I think all can agree that it is a waste of time for a human to
> > look
> > for basic codestyle errors
> > such as whitespace and friends.
> 
> I agree.  There is good editor support and people should use
> this.  ;-)
> 
> > Lots of projects have checkpatch scripts or similar that do these
> > checks automatically and policy that all contributors
> > are supposed to "don't post the patch until checkpatch runs
> > cleanly".
> > It works well for those projects. I am certain
> > it would work well for samba too.
> > 
> > I just don't see the problem or controversy.
> 
> The problem is that the *initial* proposal is not for a checkpatch
> script that people can run *before* submitting their patch.  The
> proposal is for something that runs at the submission gate.  When
> that
> thing gets it wrong then you can't push your code.  We already have
> problems just before releases when nobody can push anything because
> we
> have too many flakey tests.  We don't need another barrier.
> 
> Other proposals are for tests run aganst existing code with a strong
> suggestion that all existing code should be cleaned up.  That's not a
> good use of time.
> 
> If there is a clear proposal for a script that people run themselves
> then I think that is a reasonable starting point because it can be
> tested and confidence can be gained before it gets in the way of
> submitting patches.
> 
> In terms of the current whitespace complaints, Douglas' proposal to
> use
> "git <pick-one-of-several> --check" is the best because we don't have
> to
> write or maintain a single line of code.  Well-tested code already
> exists so we should use that.
> 
> Swen's suggestion of using the kernel's checkpatch is reasonable, but
> the code will require modifications.  It doesn't cope with more than
> one file in a patch, 
No, what makes you think so ?

[swen at localhost samba]$ git diff  |/usr/src/kernels/4.14.18-300.fc27.x86_64/scripts/checkpatch.pl --no-signoff --no-tree -
WARNING: line over 80 characters
#9: FILE: ctdb/common/ctdb_io.c:188:
+    sdflksdfklsdf				sdfläkfdgkldfgkldflgkdflgdflgkdfglkkdfglkdfglk

WARNING: please, no spaces at the start of a line
#9: FILE: ctdb/common/ctdb_io.c:188:
+    sdflksdfklsdf^I^I^I^Isdfläkfdgkldfgkldflgkdflgdflgkdfglkkdfglkdfglk$

WARNING: line over 80 characters
#21: FILE: ctdb/common/tunable.c:218:
+    sdflksdfklsdf				sdfläkfdgkldfgkldflgkdflgdflgkdfglkkdfglkdfglk

WARNING: please, no spaces at the start of a line
#21: FILE: ctdb/common/tunable.c:218:
+    sdflksdfklsdf^I^I^I^Isdfläkfdgkldfgkldflgkdflgdflgkdfglkkdfglkdfglk$

total: 0 errors, 4 warnings, 14 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.
[swen at localhost samba]$ git diff  |/usr/src/kernels/4.14.18-300.fc27.x86_64/scripts/checkpatch.pl --no-signoff --no-tree -
ERROR: trailing whitespace
#9: FILE: ctdb/common/ctdb_io.c:188:
+    sdflksdfklsdf^I^I^I^Isdfläkfdgkldfgkldflgkdflgdflgkdfglkkdfglkdfglk   $

WARNING: line over 80 characters
#9: FILE: ctdb/common/ctdb_io.c:188:
+    sdflksdfklsdf				sdfläkfdgkldfgkldflgkdflgdflgkdfglkkdfglkdfglk   

WARNING: please, no spaces at the start of a line
#9: FILE: ctdb/common/ctdb_io.c:188:
+    sdflksdfklsdf^I^I^I^Isdfläkfdgkldfgkldflgkdflgdflgkdfglkkdfglkdfglk   $

WARNING: line over 80 characters
#21: FILE: ctdb/common/tunable.c:218:
+    sdflksdfklsdf				sdfläkfdgkldfgkldflgkdflgdflgkdfglkkdfglkdfglk

WARNING: please, no spaces at the start of a line
#21: FILE: ctdb/common/tunable.c:218:
+    sdflksdfklsdf^I^I^I^Isdfläkfdgkldfgkldflgkdflgdflgkdfglkkdfglkdfglk$

total: 1 errors, 4 warnings, 14 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

NOTE: Whitespace errors detected.
      You may wish to use scripts/cleanpatch or scripts/cleanfile

Your patch has style problems, please review.

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

As you can see I "poluted" 2 files and used standard kernel checkpatch against it.

Cheers Swen.