Re: [coreboot] Proposing a change to Coding Style

[Julien Viard de Galbert]

> Le 16 mai 2018 à 17:15, Patrick Georgi via coreboot  a écrit :
>
> Hi everybody,
>
> after just running into an issue on the EC code base, I hereby propose that
> going forward, we should always wrap conditional blocks in braces, even
> one-liners.
> That is:
>
> if (foo) {
>   bar();
> }
>
> instead of
>
> if (foo)
>   bar();
>
> It doesn't hurt too much but saves us from accidentally adding baz() after
> bar(), forgetting to add the - now required - braces. If we get rough
> consensus over this, I'd change Coding_Style to match.
>
> Thoughts?
> Patrick
> -- 
> Google Germany GmbH, ABC-Str. 19, 20354 Hamburg
> Registergericht und -nummer: Hamburg, HRB 86891, Sitz der Gesellschaft:
> Hamburg
> Geschäftsführer: Paul Manicle, Halimah DeLaine Prado

Hello everyone,

Not that my opinion matter most (I’m just a newbie after all) but:

I really like this proposal. And agree that this is error prone. I also like the 
} else { option.

Remember that the script that check commits is enforcing removing the braces,
I was hit by that when porting the code for denverton.
As a side note, There is also a 80 column limit, I was told that printf format 
string can exceed the limit but the script complains…

Just saying that the script should be updated if the policy is changed.

Best Regards

Proposing a change to Coding Style

[Nico Huber]

On 18.05.2018 14:54, Julien Viard de Galbert wrote:
> I really like this proposal. And agree that this is error prone. I also like the 
> } else { option.

Just to make sure nobody gets this wrong:

} else {

on a single line is already mandated by our Coding Style. It's not an
option (unless you have no braces at all).

Nico

Proposing a change to Coding Style

[ron minnich]

https://review.coreboot.org/#/c/coreboot/+/26368/

On Fri, May 18, 2018 at 5:55 AM Julien Viard de Galbert <
[e-mail]> wrote:
> Le 16 mai 2018 à 17:15, Patrick Georgi via coreboot 
> a écrit :
>
> Hi everybody,
>
> after just running into an issue on the EC code base, I hereby propose that
> going forward, we should always wrap conditional blocks in braces, even
> one-liners.
> That is:
>
> if (foo) {
>   bar();
> }
>
> instead of
>
> if (foo)
>   bar();
>
> It doesn't hurt too much but saves us from accidentally adding baz() after
> bar(), forgetting to add the - now required - braces. If we get rough
> consensus over this, I'd change Coding_Style to match.
>
> Thoughts?
> Patrick
> --
> Google Germany GmbH, ABC-Str. 19, 20354 Hamburg
> Registergericht und -nummer: Hamburg, HRB 86891, Sitz der Gesellschaft:
> Hamburg
> Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
>
> Hello everyone,
>
> Not that my opinion matter most (I’m just a newbie after all) but:
>
> I really like this proposal. And agree that this is error prone. I also
> like the
> } else { option.
>
> Remember that the script that check commits is enforcing removing the
> braces,
> I was hit by that when porting the code for denverton.
> As a side note, There is also a 80 column limit, I was told that printf
> format
> string can exceed the limit but the script complains…
>
> Just saying that the script should be updated if the policy is changed.
>
> Best Regards