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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to