[Lldb-commits] [lldb] [llvm] [lldb][telemetry] Implement LLDB Telemetry (part 1) (PR #119716)
Vy Nguyen via lldb-commits
lldb-commits at lists.llvm.org
Wed Dec 18 11:48:32 PST 2024
oontvoo wrote:
> > Pavel's suggestion was to split up the patch to make reviewing easier
>
> It was, but this wasn't the kind of separation I had in mind. For me as least, it's very hard to review features in these "horizontal" slices. It's also unclear how (if ever) will this code be tested.
>
> I think it be much better to slice this up vertically, so that (e.g.) the `CommandTelemetryInfo` was added in the same patch that was actually using it.
P.S: I've updated this patch to remove all of the additional base classes of TelemtryInfo, keeping only the LldbBaseTelemtryInfo. Similarly, removed most of the `log...*()` methods from this patch.
PTAL.
https://github.com/llvm/llvm-project/pull/119716
More information about the lldb-commits
mailing list