search

nholthaus / units

a compile-time, header-only, dimensional analysis and unit conversion library built on c++14 with no dependencies.
http://nholthaus.github.io/units/
MIT License
952 stars 135 forks source link

clang-format support #117

Open JohelEGP opened 6 years ago

JohelEGP commented 6 years ago

Would you like me to build a .clang-format file for formatting the header? I'd choose the options that keep intact as much code as possible. This would be with done with Clang 6 and the HEAD of branch v3.x.

nholthaus commented 6 years ago

If you're willing, totally. Can we add CI check for conformance?

On Thu, May 10, 2018, 10:25 PM Johel Ernesto Guerrero Peña < notifications@github.com> wrote:

Would you like me to build a .clang-format file for formatting the header? I'd choose the options that keep intact as much code as possible. This would be with done with Clang 6 and the HEAD of branch v3.x.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nholthaus/units/issues/117, or mute the thread https://github.com/notifications/unsubscribe-auth/AJ2HHxWphg30X-sAFE9dkkzrX4lBkcpoks5txPa1gaJpZM4T6z5T .

JohelEGP commented 6 years ago

I am ashamed to say that I have yet to enable CI in any of my repositories, so I have no experience with that. EDIT: And rather than checking for conformance, it'd be better if the CI ran clang-format on the commits and, if possible, squashed them on the PRs, because different Clang versions format code slightly different even with the same .clang-format configuration file.

nholthaus commented 6 years ago

Once you write the format, I'll do the CI integration.

JohelEGP commented 6 years ago

I'd choose the options that keep intact as much code as possible.

This has proven hard. I'm reducing its scope to the operators, which are mostly formatted uniformly.

I'm thinking of setting the column limit to 120. Currently, it is unbound, and that makes code review hard. I read somewhere that most reviews are done in diffs. The commits being accessible to everyone (and Github not adding a column limit), I think it'd be a great addition that anyone reviewing is not turned off by the overly long lines which require horizontal scrolling.

I'm prioritizing this so that I don't have to mess with whitespace anymore.

JohelEGP commented 5 years ago

To ease code review, as above, I think https://github.com/nholthaus/units/blob/5483589322daad285eedd4e2822058ba4e6640d8/.clang-format#L58 should be changed to Never. It seems that my browser renders the tabs with 8 characters, so the diffs don't show the intended 4 spaces that the tab ought to represent. As monospaced spaces are more uniform and won't vary per browser, I think this is the correct path.