[PATCH] Fix the build

[Michael Adam]

On 2016-09-02 at 20:05 +1000, Martin Schwenke wrote:
> On Fri, 2 Sep 2016 10:06:56 +0200, Michael Adam <obnox at samba.org> wrote:
> 
> > On 2016-09-02 at 09:34 +0200, Volker Lendecke wrote:
> 
> > > Review appreciated!
> > > 
> > > Looking at commit 1c636532874da from a few weeks ago I begin to question
> > > the value of our README.Coding file. I've asked a few times to fix
> > > patches to follow the 80-column rule, I even provided patches to assist.
> > > 
> > > There's a reason why we have this rule: It's not that we are all sitting
> > > at 3270 or vt100 terminals. We want to avoid arbitrarily deeply nested
> > > control structures. It might be more work, but well-named factored
> > > out subfunctions foster unterstanding of complex code. Looking at
> > > dsdb_garbage_collect_tombstones(), we have four (!!)  levels of nested
> > > for-loops. One line I've just come across almost touches twice the
> > > 80-columns with its length of 157 chars.
> > > 
> > > So, shall we drop the README.Coding section on 80 chars, as it is not
> > > generally seen as worthwhile following?  
> > 
> > 1000 times no!
> > 
> > We should rather have a check script that validates the patch.
> > 
> > e.g. glusterfs does this:
> > 
> > https://github.com/gluster/glusterfs/blob/master/rfc.sh
> > 
> > which is used to submit patches, calls
> > 
> > https://github.com/gluster/glusterfs/blob/master/extras/checkpatch.pl
> > 
> > We could include something similar in our autobuild receive hook
> > on the autobuild server. So that patches that don't adhere are
> > rejected. (A forceful override can be discussed.)
> 
> I think that would be bad too.  If you have a DEBUG message that is
> needfully long then which is better?
> 
> * Have a line 85 characters long and exceed the 80 character limit?
> 
> * Split the message so that when someone greps for it they can't easily
>   find it?
>
>   I've been frustrated by this many times.


Well, this splitting happens in a lot of places in the code.
And that this disturbs grep-ability of dbg msgs is possibly one
of the very few legitimate reasons for granting an exception.

But if it would exceed the 80 by too much, I'd still
prefer splitting the strings.

The script should have a certain generosity.
E.g. warn between 80 and 89 chars and fail starting at 90.

> I understand that we want to limit nesting to help control complexity,
> but there are cases where it makes sense to go beyond 80
> characters...  :-)

There are rare cases where an exception can be justified.
But for the majority of violations, it's just laziness.
And the pros for roughly adhering to the rules outweigh
the cons by far, imho.

After a trial period we may establish overrides.
E.g. by stigmatizing tags in the commit message
As in "override-line-lenght", etc. :-)

Just a few thoughts...

Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 163 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160902/07b74fe3/signature.sig>