-
Notifications
You must be signed in to change notification settings - Fork 2
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 formatting of files in place #11
Conversation
f36e347
to
d6adaa9
Compare
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.
Works like a charm. I didn't review too much in details, but LGTM
This commit renames and redefines the `clang_format_aspect` to take `clang-format` options and execution requirements. It also defines a fix aspect which modifies files in place with `clang-format`, removing the need for an update script and subsequent need to determine the `bazel` process. resolves #5 Change-Id: I90d6c95e921d6ab9565bfe5318635901d3de8ad1
d6adaa9
to
2655fa3
Compare
command = """ | ||
set -euo pipefail | ||
|
||
test -e .clang-format || ln -s -f {config} .clang-format |
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.
3rd party should really avoid doing any symlinks especially using -f
flag. Clang-tidy supports clang-format -style=file:<path/to/config/file> ...
notation since 14.0.0.
For example one can do:
clang-format -style=file:/home/username/projects/someproject/.clang-format -i main.cpp
Would you fix that it might worth to mention minimal clang-format
version on README site
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.
I forgot that sandboxing is disabled with the new fix aspect. Sorry!
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.
No problem :-) Should be very straightforward to fix that
This commit renames and redefines the
clang_format_aspect
to takeclang-format
options and execution requirements. It also defines a fixaspect which modifies files in place with
clang-format
, removing theneed for an update script and subsequent need to determine the
bazel
process.
resolves #5
Change-Id: I90d6c95e921d6ab9565bfe5318635901d3de8ad1