JDevlieghere accepted this revision as: JDevlieghere. JDevlieghere added a comment. This revision is now accepted and ready to land.
A few small nits, but overall this LGTM. ================ Comment at: lldb/include/lldb/API/SBData.h:16 +class ScriptInterpreter; +} + ---------------- The linter is right. Did you clang-format your patch? It should add it automatically. ================ Comment at: lldb/source/Interpreter/ScriptInterpreter.cpp:31 + Debugger &debugger, lldb::ScriptLanguage script_lang, + lldb::ScriptedProcessInterfaceSP scripted_process_interface_sp) + : m_debugger(debugger), m_script_lang(script_lang), ---------------- If you gave the scripted_process_interface_sp a default value (`scripted_process_interface_sp = {}`), you could skip the overload, unless that prevents you from forward declaring the interface in the header? ================ Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp:94 + + // right now we know this function exists and is callable.. + PythonObject py_return( ---------------- I know you copied this code but let's turn this and the other comments in this file into full sentences. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95711/new/ https://reviews.llvm.org/D95711 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits