[Lldb-commits] [PATCH] D33674: Implementation of Intel(R) Processor Trace support for Linux

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

I still see some room for improvement, but some of those require some infrastructure improvements, like being able to construct llvm::StringError (or equivalent easily), which I guess will have to wait until I fix those. I think this patch has gone on long enough.

In https://reviews.llvm.org/D33674#790681, @ravitheja wrote:

> >> Regarding the testing strategy, we have tests at two levels, one at the SB API level and the other at the tool level.
> > 
> > Cool, are you going to put some of those tests in this patch?
>
> The problem with uploading tests is that they need minimum Broadwell machine and a newer Linux kernel (> 4.2 something )


Some kind of test is better than nothing, so I would still recommend you go ahead with the test. This wouldn't be the first piece of functionality without a proper test, but I hope you realize it is in your interest to have the functionality tested as much as possible -- people will be refactoring this code, and code around it -- if it's not tested, it will be broken sooner or later. In any case, I am going to leave that up to you.


https://reviews.llvm.org/D33674