Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C++ coding style is not documented #1273

Open
sbc100 opened this issue Dec 19, 2019 · 5 comments
Open

C++ coding style is not documented #1273

sbc100 opened this issue Dec 19, 2019 · 5 comments

Comments

@sbc100
Copy link
Member

sbc100 commented Dec 19, 2019

Ran into this issue when reviewing some recent additions.

Currently the only documentation I could find was that we specify BasedOnStyle: Chromium in .clang-format. This implicitly locks down a lot of things which good, more explicit docs would be good.

The specific issue we ran into was the wabt uses a style that requires curly braces around conditional blocks, but chromium, which inherits from google style allows from single line conditional, and conditional without curlys:
https://google.github.io/styleguide/cppguide.html#Conditionals
In general, curly braces are not required for single-line statements

We should be specific if we want to enforce this style.

@sbc100 sbc100 changed the title Coding style is not documented C++ coding style is not documented Dec 19, 2019
@sbc100
Copy link
Member Author

sbc100 commented Dec 19, 2019

@binji @aardappel

@binji
Copy link
Member

binji commented Jan 9, 2020

Yeah, I had a warning for missing curly braces I think, but maybe it's not triggering anymore...? In any case, it would be better to be clear about this. However, AFAIK it's not possible in the .clang-format file, since it generally only adds/removes whitespace.

@sbc100
Copy link
Member Author

sbc100 commented Jan 9, 2020

What do you think about relaxing the style to match chomium's :)

@sbc100
Copy link
Member Author

sbc100 commented Jan 9, 2020

Should we add a paragraph about the style guidelines to Contributing.md?

@binji
Copy link
Member

binji commented Jan 9, 2020

IIRC, I switched to requiring braces because it seemed like most folks preferred it.

Contributing.md seems like a reasonable place, yeah.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants