[PATCH] Fix the build

Martin Schwenke martin at meltin.net
Fri Sep 2 10:05:47 UTC 2016


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.

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...  :-)

peace & happiness,
martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160902/848f810d/attachment.sig>


More information about the samba-technical mailing list