teemperor added a comment.

This LGTM, but `shlex.join` is actually Py3 exclusive and I don't think there 
is a good Py2 replacement. I think we're just in time for the Py2->3 migration 
according to the timeline Jonas posted last year 
<https://lists.llvm.org/pipermail/lldb-dev/2020-August/016388.html>, so let's 
use this patch to actually do that? Then we can also get rid of all the `six` 
stuff etc.

Let's see if Jonas has any objections against dropping Py2 with this, otherwise 
this is good to go.



================
Comment at: lldb/test/API/test_utils/TestBaseTest.py:1
+"""
+Test TestBase test functions.
----------------
Could we move this file into `test_utils/build` or some other subdir? Then I 
can also make the few other 'test'-tests their own subdir of `test_utils` 
(which seems like a good place for all of this).


================
Comment at: lldb/test/API/test_utils/TestBaseTest.py:18
+
+    def trace(self, *args, **kwargs):
+        io = six.StringIO()
----------------
I think a comment that this overrides the normal test `trace` method would be 
nice (I wish Python had some builtin thing for indicating overrides...)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112212/new/

https://reviews.llvm.org/D112212

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to