[Lldb-commits] [PATCH] D57275: [testsuite] Remove seven dependency

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

I don't have a strong preference between using a separate module for interopability between python versions and implementing the functions as part of dotest if there are few of them, but I have to agree with @zturner that if we have more than a couple of functions, it makes sense to add them to an explicit module.

Incidentally, I think we do have more than a couple of functions but they are not in the module right now - most likely because people who added them (me included) didn't know any better.



================
Comment at: packages/Python/lldbsuite/support/seven.py:18
-                    shell=True,
-                    universal_newlines=True))
-        except subprocess.CalledProcessError as e:
----------------
I suspect that if you end up using your implementation, you'll have to add universal_newlines=True, so that it all works correctly across platform (and on Windows)


================
Comment at: packages/Python/lldbsuite/test/dotest.py:51
 
+def get_command_output(cmd):
+  try:
----------------
This version of get_command_output no longer uses six for python 2, does it still work correctly for both python 2 and python 3?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57275