[PATCH] Fix the build

Simo simo at samba.org
Fri Sep 2 15:24:45 UTC 2016


On Fri, 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.
> 
> 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...  :-)

I found those cases rarely exist and breaking them is not a big deal.
Yes the grep message thing is real, but so often messages are composed
by substrings anyway so you already have to use fuzzy logic to find
out 
where the actual message is for grepping.
Besides, the 80 column rule *really* helps debug messages too, you do
not want overly long debug messages in debug logs either! It makes it
hard to read them.

In general overly long lines makes code harder to parse (I have actual
difficulty reading long lines, and line wrapping is horrible for
following code flow).

Simo.




More information about the samba-technical mailing list