Whitespace and bullying

[Martin Schwenke]

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, which can obviously be got around by having it
look at individual commits... but this entails a wrapper that we
have to maintain. In some cases it asks if you:

  WARNING: added, moved or deleted file(s), does MAINTAINERS need
  updating?

and there's no way of turning that off.  So, we end up with a fork to
maintain.

Simo's scripts blow up on some reasonable looking commits and I can't
work out why.

I'm sure there is a way forward but I don't think it starts by
putting another barrier at the submission gate.  I think it starts by
coming up with something that we can build confidence in.

However, we have a catch-22:

* We can't start with a check at the submission gate because we need to
  be confident that it works well.

* Adding an optional, user-run script won't fix the problem if people
  don't run it.

  I repeat:

  There is already good editor support to avoid the particular problems
  we're talking about.  This is optional and is not used by everyone.
  If everyone used it then we would not be having this conversation.

peace & happiness,
martin