[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

Jonas Toth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 13 13:02:15 PDT 2018


JonasToth added inline comments.


================
Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:2
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+
----------------
hugoeg wrote:
> JonasToth wrote:
> > hugoeg wrote:
> > > hokein wrote:
> > > > nit: please make sure the code follow LLVM code style, even for test code :)
> > > what is this in reference too?
> > > Will the test still work if I wrap the CHECK MESSAGE lines?
> > CHECK-MESSAGE can be on one line, even if its longer (that is common in the clang-tidy tests).
> > 
> > But dont use many empty lines and respect naming conventions and run clang-format over the code (except there is a valid reason that the formatting would infer with the tested logic).
> Do my function names have to be verbs, they're not doing anything.
> 
> I could rename InternalFunction to something like InternallyProcessString annd StringFunction to process String
> Would this be preferred?
It helps if you somehow show the "topic of the function". But I wrote some tests, that did not strictly follow and they passed review too ;)

Foo is just too generic, sth like `DirectAccess`, `FriendUsage`, `OpeningNamespace` or so is already telling and I guess good enough :)


https://reviews.llvm.org/D50542





More information about the cfe-commits mailing list