On 4/26/16 8:28 AM, Christopher Collins wrote:
On Sun, Apr 24, 2016 at 10:08:09AM -0700, Sterling Hughes wrote:
Hi,
As we start to bring on new contributors, and operate as a project, its
increasingly important that we document and agree upon coding standards.
I think we've done a good job of maintaining this consistency
informally, but, now we need to vote and agree on project standards.
I've taken a first stab at this and committed it to the develop branch,
folks can see it here:
https://github.com/apache/incubator-mynewt-core/blob/develop/CODING_STANDARDS.md
Thanks for putting this together Sterling. I think it looks great. My
opinion is that a coding standards document should not be overly
prescriptive. Everyone has his own set of coding pet peeves; I suggest
we try to keep those out of this document and keep it as short as
possible. Otherwise, people won't adhere to the document, or they will
just hate writing code and they won't contribute as much.
For me, the important rules are:
* Maximum line length
* Brace placement
* Typedefs
* All-caps macros
* Compiler extensions (e.g., packed structs).
The first three are already captured; I think the others should be
addressed. I think macros should always be in all-caps for reasons that
everyone is probably familiar with. Unfortunately, I don't have a good
rule for when extensions are acceptable.
+1
I have never found a scenario where macros should be lower case. I have
found some where I thought it would make sense (e.g. min()), but in
retrospect, MIN() or an inline function would have been just fine.
I would also like to see a note about when it is OK to stray from the
conventions. There will be times (rarely) when adhering to the
standards document just doesn't make sense. "Zero-tolerance" rules
always seem to pave the road to hell :).
Finally, there is one point in particular that I wanted to address:
include guards in header files. From the document:
* ```#ifdef``` aliasing, shall be in the following format, where
the package name is "os" and the file name is "callout.h":
```no-highlight
#ifndef _OS_CALLOUT_H
#define _OS_CALLOUT_H
I am not sure I like this rule for the following two reasons:
* A lot of the code base doesn't follow this particular naming
convention.
A lot of it does :-)
* Identifiers starting with underscore and capital letter are
reserved to the implementation, so technically this opens the door
to undefined behavior.
A leading capital E is also reserved by POSIX (e.g., EINVAL). The
naming convention I use is:
H_CALLOUT_
I would not consider this something to worry about, and I don't think we
need to include a specific naming convention in the document. However,
insofar as we prescribe a naming convention, it should be one which
avoids undefined behavior.
I think we do need a naming convention here - I'm fine adopting H_*.
I'll point out that almost every POSIX or LIBC header I've ever seen
uses underscore, and I believe this is only reserved in C++ -- that
said, we need to be friendly to C++, and as you point out, we should
follow a defined behavior.
Sterling
