[Lldb-commits] [PATCH] D33674: Implementation of Intel(R) Processor Trace support for Linux
Pavel Labath via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Tue Jun 27 09:25:03 PDT 2017
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.
More information about the lldb-commits