kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land.
================ Comment at: clang-tools-extra/clangd/support/Trace.cpp:91 public: - JSONSpan(JSONTracer *Tracer, llvm::StringRef Name, llvm::json::Object *Args) + llvm::json::Object Args; + JSONSpan(JSONTracer *Tracer, llvm::StringRef Name) ---------------- move this near other fileds? ================ Comment at: clang-tools-extra/clangd/support/Trace.h:86 + beginSpan(llvm::StringRef Name, + llvm::function_ref<void(llvm::json::Object *)> RequestArgs); // Called when a Span is destroyed (it may still be active on other threads). ---------------- nit: at first i parsed request in `RequestArgs` as a verb (e.g. `i request arguments`), which was confusing a little bit. Hence the comment also didn't make much sense, I only managed to error correct after seeing the usage :D Maybe something like `SetRequestArgs` to emphasize that one provides request arguments using this lambda rather than receiving them? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89135/new/ https://reviews.llvm.org/D89135 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
