> On Apr 25, 2016, at 9:22 PM, [email protected] wrote:
>
>
> 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.
+ 1.
> 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?
Please god say no :-)
> 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() )
I thought this was mentioned?
> 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!!
Good luck getting others to change this :-) I would be fine personally.
> 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.
It is just me personally, but the switch/case above is hard for me to read
(because of the {} around the guts of the case).
So I would vote -1 for that.
>
>
>
>
> 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
>
