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