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

Fix Klockwork compilation warning #410

Closed
wants to merge 1 commit into from

Conversation

chenhayat
Copy link
Contributor

No description provided.

Copy link
Contributor

@vitaut vitaut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, just one nitpick regarding code style.

{
visit_any_int(value);
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please follow Google style guide: https://google.github.io/styleguide/cppguide.html

@foonathan
Copy link
Contributor

foonathan commented Nov 5, 2016

@vitaut Could you perhaps add a .clang-format file? It would make contributing much easier.

Edit: For example this commit: d8867a2
I wrote the changes, compiled, ran tests, reformatted and accidentally deleted a '>'.
With clang-format autoformatting, this would not have happened.

@vitaut
Copy link
Contributor

vitaut commented Nov 6, 2016

@foonathan What will be the benefit of having .clang-format in the project itself compared to say passing -style=google to clang-format?

@foonathan
Copy link
Contributor

Oh, so you're using the google style without modification? Did not know that.

@vitaut
Copy link
Contributor

vitaut commented Nov 6, 2016

Yes, with the exception of snake_case instead of CapitalizedWords for method names, but this doesn't affect formatting. I should probably document it somewhere.

@foonathan
Copy link
Contributor

Yes, you should document it.

But running clang-format over the source files will also change some other stuff like the indentation of your #defines when in #ifdef, break lines differently, etc.

@vitaut
Copy link
Contributor

vitaut commented Nov 6, 2016

@chenhayat Merged in 05ba3e7, thanks!

@foonathan I've documented the coding style in https://github.com/fmtlib/fmt/blob/master/CONTRIBUTING.rst. clang-format does mess up preprocessor indentation, so I am a bit reluctant of applying it globally. Hopefully they'll fix https://llvm.org/bugs/show_bug.cgi?id=17362 and until then I suggest limiting it to formatting patches.

@vitaut vitaut closed this Nov 6, 2016
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

Successfully merging this pull request may close these issues.

3 participants