Add clang-format

[GretaCB]

Per #37:

• Add clang-format script
• Format changes after running the script
• Add contributing.md with how to run format
• Modifying a single travis job to run the ./scripts/format.sh. If the exit code is non-zero then it will fail, which will remind developers to run the ./scripts/format.sh locally before pushing to ensure we stay consistent going forward.

cc @springmeyer @daniel-j-h

[codecov-io]

Codecov Report

Merging #56 into master will increase coverage by 0.23%.
The diff coverage is 94.44%.

@@            Coverage Diff             @@
##           master      #56      +/-   ##
==========================================
+ Coverage   94.32%   94.55%   +0.23%     
==========================================
  Files           8        8              
  Lines         141      147       +6     
==========================================
+ Hits          133      139       +6     
  Misses          8        8

Impacted Files	Coverage Δ
src/module.cpp	100% <ø> (ø)	⬆️
src/object_async/hello_async.hpp	0% <0%> (ø)	⬆️
src/object_sync/hello.hpp	0% <0%> (ø)	⬆️
src/standalone/hello.cpp	100% <100%> (ø)	⬆️
src/module_utils.hpp	100% <100%> (ø)	⬆️
src/object_sync/hello.cpp	96.77% <100%> (ø)	⬆️
src/standalone_async/hello_async.cpp	94.59% <92%> (+0.3%)	⬆️
src/object_async/hello_async.cpp	95.58% <96.15%> (+0.2%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d1ebcd2...a2338ec. Read the comment docs.

[springmeyer]

👍

We can always tweak the .clang-format in the future. This is a great start to getting more consistency in place.

[daniel-j-h]

Flagging here: we got burned a couple of times in osrm because of slight differences in clang-format versions, as in: someone formats with clang-format-3.8 then another dev formats with clang-format-4.0 and we get annoying formatting ping-pong.

What we did was to 1/ use a fixed version for clang-format from mason (this can be different than the clang compiler we're compiling with) and 2/ provide a base line .clang-format config based on that version (clang-format -style=X -dump-config) and tune that to your needs.

[GretaCB]

Thanks for the input @daniel-j-h 🙌

[GretaCB]

Assuming the format version mirrors the clang compiler version?

this can be different than the clang compiler we're compiling with

And sounds like it doesnt have to match, but generally 3.8 and 4.0 are the compiler versions?

[springmeyer]

Also want to chime in with a thank you @daniel-j-h. I recall that ping-pong issue and that factored into the design of node-cpp-skel, in the sense that we provide a single script ./scripts/setup.sh to make it easy to use a consistent clang++ version (currently v4.0.0 via

node-cpp-skel/scripts/setup.sh

Line 7 in dbef468

export MASON_LLVM_RELEASE="${MASON_LLVM_RELEASE:-4.0.0}"

). The formatting script currently uses this version by requesting clang++ from the setup.sh (dbef468#diff-8cfa73a13763a5d60cbae684e6ba1015R17).

So, I'm imagining all developers will run ./scripts/format.sh locally and that is also what we'll run remotely on travis. This will ensure we avoid ping-pong. It also means that when we upgrade that version we'll have some potential disruption if newer clang++ versions behave differently. If figure this is an acceptable tradeoff.

2/ provide a base line .clang-format config based on that version (clang-format -style=X -dump-config) and tune that to your needs.

Great to know this command. I just played around in node-cpp-skel with ./mason_packages/.link/bin/clang-format -style=llvm -dump-config > .clang-format and seeing the differences. @GretaCB My feeling is that we should not worry about changing the current .clang-format you pulled from variant for now. We can entertain adopting a different code style in the future, but this should be discussed more widely at Mapbox. The important thing now is to get the structure in place.

[springmeyer]

Just checked to see if upgrading to clang++ 4.0.1 formats the code differently than clang 4.0.0 given the same .clang-format. It does not 🎉 . So, we should plan to upgrade clang++ to the newest from mason (mapbox/mason#452) in another PR. Ticketed this at #58

[GretaCB]

Travis is also set 🎉

[springmeyer]

This looks good. I feel like an improvement would be to strip way even more details to focus this job exclusively on formatting. It could do language: generic and override install: to just run the setup.sh and not the make step - then this would run even faster and would not need the custom env.

[springmeyer]

Looks good to merge pending contributing.md

[springmeyer]

One failure on travis unrelated to this PR that happened on OS X:

Downloading https://s3.amazonaws.com/mason-binaries/osx-x86_64/clang++/4.0.0.tar.gz
curl: (56) SSLRead() return error -36
bin/clang-4.0: (Empty error message)
tar: Error exit delayed from previous errors.

Looks like travis-ci/travis-ci#7636, which appears unsolved, except that some users have switched to wget. No action that we need to take in node-cpp-skel, but mentioning here what I learned nevertheless.