[PATCH] D49116: Setup clang-format as an Arcanist linter

[Siddhartha Bagaria via Phabricator via llvm-commits]

starsid marked 3 inline comments as done.
starsid added inline comments.


================
Comment at: utils/arcanist/clang-format.sh:13
+
+if ! hash clang-format-diff >/dev/null; then
+  # advice severity level is completely non-disruptive.
----------------
mehdi_amini wrote:
> I have clang-format installed, I can run `git clang-format` as well, but I don't have a `clang-format-diff` executable and I don't even know how to get it.
> 
> However I see `clang/tools/clang-format/clang-format-diff.py` in the repo, can we call this directly?
Sure, `clang-format-diff` was a suggestion from the last code review.  I don't think `git-clang-format` was available at the time. Please feel free to send an update to people to get it approved and landed.


================
Comment at: utils/arcanist/clang-format.sh:43
+  arc_base_commit=$(arc which --show-base)
+  # An alternative is to use git-clang-format.
+  git diff -U0 --no-color "${arc_base_commit}"| clang-format-diff -style LLVM -i -p1
----------------
mehdi_amini wrote:
> Can we just use  `git clang-format` indeed?
I think so. Please send a review.


================
Comment at: utils/arcanist/clang-format.sh:46
+else
+  svn diff --diff-cmd=diff -x -U0 "${src_file}" | clang-format-diff -style LLVM -i
+fi
----------------
mehdi_amini wrote:
> This does not seem relevant now that we're on GitHub.
Can be replaced with an error message I believe.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49116/new/

https://reviews.llvm.org/D49116