ilya-biryukov added a comment. Last drop of NITs. After they're fixed, this change is ready to land!
================ Comment at: unittests/clangd/XRefsTests.cpp:282 + const char *HeaderContents = R"cpp([[]]int a;)cpp"; + Annotations HeaderAnnoations(HeaderContents); + FS.Files[FooH] = HeaderAnnoations.code(); ---------------- There's a typo, should be `HeaderAnnotations` ================ Comment at: unittests/clangd/XRefsTests.cpp:302 + Locations = ExpectedLocations->Value; + EXPECT_TRUE(!Locations.empty()); + ASSERT_EQ(Locations[0].uri.file, FooH); ---------------- NIT: one assertion using `EXPECT_THAT` will read better and produce more helpful messages in case of failures: ``` // Create FooHUri to avoid typing URIForFile everywhere. auto FooHUri = URIForFile{FooH.str()}; EXPECT_THAT(Locations, ElementsAre(Location{FooHUri, HeaderAnnoations.range()})); ``` ================ Comment at: unittests/clangd/XRefsTests.cpp:328 + Locations = ExpectedLocations->Value; + EXPECT_TRUE(Locations.empty()); + ---------------- NIT: Use `EXPECT_THAT` here too: `EXPECT_THAT(Locations, IsEmpty());` Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D38639 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits