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.
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.
* 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.
Thanks,
Chris
