-
Notifications
You must be signed in to change notification settings - Fork 254
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
Add code formatting infrastructure #397
Open
matt-gretton-dann
wants to merge
6
commits into
master
Choose a base branch
from
matt-gretton-dann/clang-format
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
matt-gretton-dann
requested review from
gnedjati,
igfoo and
Z80coder
as code owners
June 18, 2020 11:50
matt-gretton-dann
force-pushed
the
matt-gretton-dann/clang-format
branch
from
June 18, 2020 12:50
d4c07bf
to
cecd109
Compare
This adds a workflow that will automatically clang-format any code on a pull_request.
This is the automatic part of applying clang-format. Not everything may build yet.
matt-gretton-dann
force-pushed
the
matt-gretton-dann/clang-format
branch
from
June 18, 2020 14:32
cecd109
to
9ac2541
Compare
Clang format rearranged some of the header includes which highlighted some build issues.
The clangformat auto-applied patch produced some untidily formatted comments. This commit tidies up the ones I spotted.
matt-gretton-dann
force-pushed
the
matt-gretton-dann/clang-format
branch
from
June 18, 2020 14:44
9ac2541
to
5648898
Compare
As I said in the previous thread: tabs make changelogs look bad by default and thus require additional user work. Unless you customize the tab size in Git diffs, they won't be displayed as you intended. |
zebmason
suggested changes
Jul 9, 2020
ColumnLimit: 100 | ||
IndentWidth: 2 | ||
Language: Cpp | ||
Standard: Cpp11 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't Standard be Cpp14?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a .clang-format file to format code at the top-level and enforces it using a GitHub Action.
On Windows modern versions of Visual Studio should pick up the .clang-format file as well and automatically format code when saved.
The formatting choices made are based on Microsoft's defaults for Visual Studio. Of interest though will be the choices between tabs & spaces, indent width and max line length.
I have chosen tabs with an indent of 2 spaces per-tab. This is what we've had previously, but I'm interested in any other thoughts @NeilFerguson, @dlaydon, @weshinsley.
On line length - I've picked 100, as that is what the Linux Kernel has recently moved to, and my view is that I start losing context on longer lines. Again up for discussion.