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