[llvm] [llvm]Add a simple Telemetry framework (PR #102323)
Vy Nguyen via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 29 12:04:13 PDT 2024
oontvoo wrote:
> As noted elsewhere, please redo all your variable and function names to conform with the coding standards. It's surprisingly distracting having them wrong! Also, please reflow your comments to properly use the 80 character width, as it makes it a little harder to read when they are broken at somewhat arbitrary lengths.
Done - renamed and reformatted the files.
(Sorry, this was extracted from LLDB code, where the styling was a bit different)
> To help reviewers, could you add some comments (potentially doxygen style) to the classes that you've added that describe their intended purpose, along with possibly a top-level comment highlighting the overall structure and how a tool might use this telemetry code
Done - added the header comment to Telemetry.h to describe the package along with some of the important design decisions. Please let me know if that's sufficient. (Can also link the RFC there ... not sure if it's common to link a discourse thread ?)
> I think you need at least one concrete implementation that can be used to write to a raw_ostream instance, which can be used for unit testing, and which helps to illustrate the interface.
Yes -the test defined one. (I'd also noted in the header comment on why we don't have a concrete implementation)
https://github.com/llvm/llvm-project/pull/102323
More information about the llvm-commits
mailing list