Adoption of Google code style and lint tool

[royfalk]

At present we are not aligned to a specific style. The style currently recommended (linux kernel) is C and not C++. I suggest we align with Google C++. I have no specific preference for it, except that it comes with a linter that supposedly identifies issues. Looking a few years into the future, it would be nice to enforce this at the PR level.

I suggest we discuss this here or in a dedicated room at gitter and then vote.

[BenjamenMeyer]

I'm all for finding a linter that can enforce our coding style and would love to see that as part of the PR gates too, along with a script that can run it locally so devs can easily check it prior to submitting PRs. Let's resolve #481 and #483 first, then we can find a tool for this that matches those decisions.

That said, I've always had trouble finding one that works well for C and C++.

[royfalk]

That's why I keep promoting Google's style. They wrote a linter for it. In theory, we can hack it to check for another style, but life is too short.

[stephengtuggy]

@royfalk I agree with this proposal 100%.

[stephengtuggy]

"Life is too short" -- yes, indeed

[stephengtuggy]

If cpplint tries to tell us that we should be using auto in more places, then we can turn that warning off. I don't know that it will though

[BenjamenMeyer]

@stephengtuggy about the only place that auto is good for is with iterators where you would otherwise have things like x<y>::iterator or x<y>::const_iterator - which can be both lengthy, and cause issues with maintenance so it's one place where it becomes very convenient; so there is a place for it, but it's rather limited.

I've known some folks in the past that wanted to use auto for every variable (and argued from a standards perspective that it should be), which is just wrong.

Before saying more I'll read through the Google C++ guides. Right now I'm working on the ISO one. I'm a slow reader so please bear with me.

[BenjamenMeyer]

@royfalk I'm assuming you mean https://google.github.io/styleguide/cppguide.html for the Google C++ Style, correct?

[BenjamenMeyer]

Raw comments: https://gist.github.com/BenjamenMeyer/aebe7fa6a06fa0ae02e22b7ed0bfed12 overview:

• in general I agree with almost all of it; most of my comments are clarifying or saying we should go above what they said. So I think we can be really close to the Google Style.
• there's a few places where we deviate - for example we will be allowing exceptions since we integrate with Python and we are only C++11.

[stephengtuggy]

@BenjamenMeyer In response to your comments:

• Yes, I agree regarding spaces vs. tabs. Let's use spaces, four of them per tab. Perhaps we could even create a .editorconfig file at the root of each project to enforce this in modern IDEs / text editors. I could do this if you guys want.
• Yes, we'll have to allow exceptions to some degree. It will probably be a good idea to minimize their use, though, like you said.
• I would like to revisit using brace initializers {}. I would like to see us both initialize every field at the point of declaration, and write our own constructors explicitly (if necessary). As far as I'm aware, these two approaches would not conflict with each other. What's more, they would allow us to visually confirm that each variable and field is initialized -- something that is extremely important, and rather lacking in the current codebase. Brace initializers ({}) would give us a consistent syntax to visually verify that everything is initialized.
• Regarding namespaces in implementation files: I'm on the fence about this. Perhaps even in implementation files, we should avoid using namespace statements. They might still have too much potential to cause problems.

I'm glad that we seem to be in agreement about just about everything else.

Thanks.

[BenjamenMeyer]

@stephengtuggy certainly we should create a repo where we can store some various editor configurations for folks to use.

1. I suggest another repo so it doesn't cloud all the other repos
2. It can be organized with a folder for each editor
3. The settings can be provided for all the different languages we use across all the repos

[stephengtuggy]

@stephengtuggy certainly we should create a repo where we can store some various editor configurations for folks to use.

1. I suggest another repo so it doesn't cloud all the other repos

I'm not sure I follow how this would "cloud" the repos. It seems to me like it would be more convenient to put the .editorconfig file at the root of each repo. (That's what I meant by "at the root of each project." Sorry if that wasn't clear.)

2. It can be organized with a folder for each editor

.editorconfig covers multiple modern editors with one file. I'm pretty sure Sublime Text supports it. I know Visual Studio and Visual Studio Code do.

However, for additional per-IDE settings (such as Visual Studio Code debugging launch configurations), we could certainly put those in per-IDE folders.

3. The settings can be provided for all the different languages we use across all the repos

The one .editorconfig file would also cover most, if not all, the different languages we use.

[stephengtuggy]

I'll file a separate issue for the .editorconfig / editor settings suggestion. In the meantime, do we have enough consensus around the Google code style to begin using cpplint?