teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed.
Sorry for the delay! > It seems like the pre-merge check failures are because I incorrectly set the > project repo when I first created the revision. I noticed in the build > commands here: > https://buildkite.com/llvm-project/premerge-checks/builds/28649#b755747b-7673-4dbc-9189-2d6bbcd1fbd3 > that the cmake command doesn't include 'lldb' as a project. I was able to > repro the error on my Debian machine using those commands, and adding lldb to > the cmake command seemed to make them go away (tbh, I'm not familiar enough > with the normal output of clang-tidy to be sure) You can ignore the clang-tidy errors as they look indeed like a setup problem on the bot. They won't prevent this from being merged. I think this looks almost ready beside some small problems (some of which are maybe a bit subjective). ================ Comment at: lldb/include/lldb/Core/FormatEntity.h:111 + const char *string = nullptr; + /// The type of the format string (see Type above) + const Entry::Type type; ---------------- You can use `\see FormatEntity::Entry::Type` and this will be linked in doxygen. ================ Comment at: lldb/include/lldb/Core/FormatEntity.h:119 + const Definition *children = nullptr; + /// Whether + const bool keep_separator = false; ---------------- Truncated comment? Also thanks for documenting this! ================ Comment at: lldb/unittests/Core/FormatEntityTest.cpp:23 + + ASSERT_EQ(d.name, "foo"); + ASSERT_EQ(d.string, nullptr); ---------------- I think you can use `EXPECT_*` in all these tests. ================ Comment at: lldb/unittests/Core/FormatEntityTest.cpp:154 +TEST(FormatEntity, LookupAllEntriesInTree) { + for (const auto &testString : lookupStrings) { + Entry e; ---------------- You could just use `llvm::StringRef` here (it's shorter and more descriptive than the `auto`). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98153/new/ https://reviews.llvm.org/D98153 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
