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

Update code style with clang-format #120

Merged
merged 8 commits into from Dec 10, 2019
Merged

Conversation

bghgary
Copy link
Contributor

@bghgary bghgary commented Dec 10, 2019

Some notes:

  • I only did this for the Library folder. I don't think we need to do it for the other files. Ideally, we should only do it for Library/Include and Library/Source.
  • Use style based on LLVM.
  • @syntheticmagus and I decided that {} initializers should not have spaces inside to match LLVM defaults.

There are some weird bugs with clang-format. I either used // clang-format off, rearranged the code, or just didn't bother. Here are some notable ones:

  • { style for initializers. They should be on their own line, but I couldn't find a way to work around it.
  • Continuations with .then is really wonky if there are multiple continuations. The style AlignAfterOpenBracket doesn't seem to work for multiple continuations.

@CedricGuillemet
Copy link
Contributor

I'd keep the clangformat style file in the root.
Otherwise, everything is fine.

@bghgary
Copy link
Contributor Author

bghgary commented Dec 10, 2019

I'd keep the clangformat style file in the root.

I don't have a strong preference, but the way .clang-format works right now is kind of bad if you have submodules that don't have .clang-format. Maybe I can add a .clang-format to Dependencies to prevent it from leaking into submodules and move the repo one to the root.

@bghgary bghgary merged commit 51470db into BabylonJS:master Dec 10, 2019
@bghgary bghgary deleted the clang-format branch December 10, 2019 21:48
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.

None yet

2 participants