My notes. Only a few strong opinions.
0) Way to go Sterling. Better sooner than later for this.
1) when I was writing the hal I wrote a hal/hal_adc.h (public API) and
mcu/hal_adc.h (private API name for the BSP to set MCU specific
parameters).
I was burned because of the #ifndef __HAL_ADC_H__. I replace them with
__HAL_HAL_ADC_H__ and __MCU_HAL_ADC_H__. Really, there was probably not
a need to have the BSP include the public API, but I believe there was
some
include path that surprised me. If we are serious about using the
directory
as a namespace, we had better include it in the header include protection.
These types of errors can be hard to detect.
2) I¹m a fan of macros in upper case only.
3) Regarding typedefs, can they be used for static functions. Seems that
this makes things readable and doesn¹t cause the harm you mention.
4) Should we be picky about the use of const?
5) any rules or naming conventions for enums?
6) any guidelines on file names. Is there a name length limit. Probably
should make names all lower case. Should the file name and function name
match for public functions? (e.g. hal_adc.h contains hal_adc_init() )
7) Muti-line comments formatted like in our apache header.
8) Any convention on line break character \?
9) I think the 79 character line length is not really helpful. I¹d rather
see slightly longer lines. I often prefer To use longer names for example
int res = hal_adc_get_resolution_mvolts(padc) to make it clear what is
going on and the units, but that may make lots of wrapping with an 80
column limit. This simple statement used 45 characters already. I know
its been standard forever, but screens are 5x wider than they used to be.
Can¹t we stretch this to 120. I hope you are reading this email with
80 columns!!
10) any other comment info like author or date at the top of the file ?
11) It always bums me out to see opposite conventions on parenthesis for
functions and other code blocks. For example suppose I do this.
void
Foo(int x)
{
/* some code with a conditional code block */
if (this or that) {
/* some code in a separate block that is conditional */
}
/* an unconditional code block with good reason */
{
uint8_t local_tmp_for_calc;
/* do a computation with some local variable and make
* it clear it has limited scope */
}
switch(condition) {
case value:
{
uint8_t a_temp_i_only_need_here;
/* do a computation with a local variable with limited
* scope */
Break;
}
}
}
I get why we want to have that lingering parenthesis on the end of the
if and switch to make the code more succinct, but it seems at odds with
the other code blocks. Maybe its my bad style, but I occasionally use
code blocks in case statements and free-standing functions to do a local
calculation with a variable that I want to make clear is only valid in a
limited scope (as opposed to declaring it at the top of the function).
This leaves my code looking inconsistent because the if and switch have
one style code block and the case, free, and function have another.
On 4/25/16, 8:48 PM, "Sterling Hughes" <[email protected]> wrote:
>
>
>On 4/25/16 8:43 PM, will sanfilippo wrote:
>> Proposed Changes:
>>
>> * A function prototype in a header file may (or should?) be a single
>>line (i.e. type and name can be on same line).
>> * Global variables should be prefaced by g_
>>
>> Comments:
>> * I dont see anything here regarding ³alignment² of various things.
>>Should we be adding these to the coding style? For example:
>>
>> This:
>> #define PRETTY (0)
>> #define VERY_PRETTY (1)
>> #define BEAUTIFUL (2)
>
>You used tabs here - so it shows unaligned in email :-), but I get the
>point and agree. I don't feel too strongly about '#define' alignment,
>but am happy to add it, I do it anyway.
>
>>
>> Not:
>> #define UGLY (0)
>> #define REALLY_UGLY (1)
>> #define HIDEOUS (2)
>>
>> ‹ or ‹
>>
>> This:
>> struct my_struct
>> {
>> int ms_foo1;
>> uint16_t ms_foo2;
>> struct qelem elem;
>> }
>>
>> Not:
>> struct my_struct
>> {
>> int ms_foo1;
>> uint16_t foo2;
>> struct qelem elem;
>> }
>
>+1 for this one.
>
>>
>> Questions:
>> * I presume that outside code not written to this style will not be
>>modified? For example, another open source project has code that we
>>adopt.
>
>We should add a note: follow the coding standards of the original source
>is my perspective.
>
>> * I presume that if not explicitly stated as ³dont do² you can do it.
>>For example, do all macros have to be uppercase? I can have either
>>MY_MACRO(x) or my_macro(x)?
>>
>
>Within reason. We can still make fun of particularly ugly code. :-)
>
>On macros, what are people's sense? I prefer to have _ALL_ my macros
>uppercased, but I didn't put that in there. I like to know what is a
>macro (upper-case), vs what is a function.
>
>Sterling
