[llvm-dev] clang-tidy pre-merge checks in Phabricator not working?

4 views
Skip to first unread message

James Henderson via llvm-dev

unread,
Nov 22, 2021, 4:06:33 AM11/22/21
to llvm-dev
Hi,

No idea who to direct this towards, but we at one point had clang-tidy linter remarks in Phabricator reviews, just like we have clang-format notes. However, I've seen several cases in recent reviews where clang-tidy hasn't complained about violations (specifically to do with function and variable name casing). Has this been disabled deliberately or is it supposed to be working, but isn't for some reason?

James

Wang, Pengfei via llvm-dev

unread,
Nov 22, 2021, 4:28:54 AM11/22/21
to jh737...@my.bristol.ac.uk, llvm...@lists.llvm.org

Did you have clang-format in your path when you committed your patch to Phabricator? I've observed no Lint remarks in Phabricator reviews if I didn’t set the path.

 

Thanks

Phoebe (Pengfei)

James Henderson via llvm-dev

unread,
Nov 22, 2021, 4:33:15 AM11/22/21
to Wang, Pengfei, llvm...@lists.llvm.org
This isn't in reviews I've been uploading, but rather reviews that I've been reviewing. I was under the impression that this was done on Phabricator's end - I use the web UI to upload patches, and don't have clang-format (or clang-tidy) in my path, but I've seen clang-format linter remarks at least on patches I've uploaded.

Mehdi AMINI via llvm-dev

unread,
Nov 24, 2021, 1:34:39 AM11/24/21
to James Henderson, Mikhail Goncharov, llvm...@lists.llvm.org
+Mikhail Goncharov who maintains a lot of this infrastructure?

_______________________________________________
LLVM Developers mailing list
llvm...@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

Mikhail Goncharov via llvm-dev

unread,
Dec 1, 2021, 5:02:31 AM12/1/21
to Mehdi AMINI, llvm...@lists.llvm.org
Hi Mehdi,

yes, I believe I have completely disabled clang-tidy checks for premerge as people were complaining about checks not being accurate.
I am planning to enable them back on an opt-in basis https://github.com/google/llvm-premerge-checks/issues/367 .

--Mikhail

James Henderson via llvm-dev

unread,
Dec 1, 2021, 6:29:31 AM12/1/21
to Mikhail Goncharov, llvm...@lists.llvm.org
Thanks. It would be helpful to have them back, I personally feel, as there have been a number of patches recently where I've had to directly ask people to fix variable and/or function name casing, if nothing else. I can't say I recall seeing many false positives, but maybe I don't work in the code areas where that would be applicable.
Reply all
Reply to author
Forward
0 new messages