[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: commit sizes, coding conventions, and back-ldif

Hallvard B Furuseth wrote: 
Howard Chu wrote:
I'm getting confused below:


Now that I think of it I could undo some of that and the patch will be
smaller, since I originally did it when I modified related code and then
later fixed an issue differently.  Or I could go all the way and
reformat everything, rename variables with abandon, and split up a
200-line function just for readability.

If you can break it up into smaller patches without too much hassle then please do so. My suggestion about just doing two commits was just in case it would be too error-prone to break it up further.

Regarding formatting, (...) 
Just offering my personal views, nothing official here.
(...)


I think you are now officially the most active contributor, so it makes
sense to try to follow your style though:-)


OK... It's worth finding out where other folks have strong preferences though.

Also, we shouldn't be considering any major reformatting while RE23 is still active. (I had been hoping we would only have to do one more RE23 release before dropping support for it and declaring RE24 stable.)

However often but not always whitespace between two parens are omitted -
e.g. "foo( bar( baz ))".  Is there some preferred guideline about that?

How about whitespace around parens that are for grouping?
E.g. "if ( (ch = getc(f)) != EOF )".

I prefer no whitespace between consecutive parens. Naked parens always
look like a syntax error to me, they're an immediate distraction that
I then have to dismiss before focusing on the point of interest.


I have the same feeling about most of the OpenLDAP spaces inside parens,
so for me it's a question of which weird and ugly style to pick.


Feel free to suggest something less ugly. ;)

So, "if (( ch = getc( f )) != EOF )"?  Or did you only mean,
not "if ( ( ch = getc( f ) ) != EOF )"?  (I notice I made a mistake
with my example, "getc(f)" should in any case have been "getc( f )".)

* Similarly generous use of {} for if statements etc, usually used even
    when enclosing just a single statement.

I prefer no brackets for single statements, but I recognize that this
leads to inconsistencies.


Yup, me too.  I tend to find briefer code more readable.  For the
same reason usually I prefer
	a = x ? y : z;
over
	if ( x ) {
		a = y;
	} else {
		a = z;
	}
I really don't get the reason for writing the latter unless the expression
gets complex - in addition to more code to read, it even makes it a bit
harder to notice that all (or both) branches modify 'a'.


Agreed.

* Indent 1 tab for continuation lines with the same paren level, e.g.
	x = foo(
		bar, baz );
(...)
OpenLDAP code often does put arguments after the '(' but to my eyes it
looks easier to read without that, in addition to making emacs happy.

I prefer as many arguments as will fit on the first line. Excessive line
breaks make it harder to grep for meaningful patterns.


Good point.  Oh well.  I do think it makes sense to group related
arguments together if that doesn't lead to more lines though, e.g.
	foo( a,
		buf, sizeof(buf) );
rather than
	foo( a, buf,
		sizeof(buf) );


OK.

* Tab-width = 4 columns.  Makes a difference for when to break lines
    (keeping line length<   80), how to align comments after code on a
    line, and to align declarations and multi-line statements.
Yes, that's pretty much carved in stone. 

Which is why I'd like to reformat the entire source tree:-( Good excuse
for changing to a style which Unix Indent and Emacs can handle though.

I guess that would provide us a permanent solution. Come up with a set of parameters for Indent and just post that on the Developer Guidelines. Sounds like a good idea.

* Usually align comments/decls to the right of code with tabs too, but
    not always.  (If we used space to align after code, it would stay
    aligned regardless of tab-width.)
I prefer tabs. 

OK.  Well, in case of the variable names in declarations I prefer a
single space, then it doesn't matter if someone who uses tab-width 8
wrote the declaration.


OK.

--
  -- Howard Chu
  Chief Architect, Symas Corp.  http://www.symas.com
  Director, Highland Sun        http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP     http://www.openldap.org/project/