JonasToth added a comment. In https://reviews.llvm.org/D50542#1205880, @hugoeg wrote:
> Anymore changes I should make or issues I should be aware of? From my side not! You still have unresolved comments that cause the revision to not be `Ready To Review` for the main reviewers, maybe that causes them to overlook it. ================ Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:2 +// RUN: %check_clang_tidy %s abseil-no-internal-deps %t + + ---------------- JonasToth wrote: > 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 :) I think the names are ok. https://reviews.llvm.org/D50542 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
