[Lldb-commits] [PATCH] D95712: [lldb/bindings] Add Python ScriptedProcess base class to lldb module

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Can you add a test for this too? I assume I would look very similar to the example. Maybe have one that checks the base class and one that overrides it with some known values.



================
Comment at: lldb/bindings/python/python-scripted-process.swig:1
+%pythoncode %{
+from abc import ABC, abstractmethod
----------------
Does this have to be a SWIG file? I guess there's nothing really special about this file except that it should end up in the `lldb` module? 


================
Comment at: lldb/bindings/python/python-scripted-process.swig:22
+
+    ### Main funcitonnalities
+    @abstractmethod
----------------
I don't think this adds much. I think it would be more valuable to have pydoc style comments on the methods. 


================
Comment at: lldb/bindings/python/python-scripted-process.swig:28
+    @abstractmethod
+    def get_memory_region_at_index(self, idx: int) -> lldb.SBMemoryRegionInfo:
+        pass
----------------
Type annotations are Python 3 only. We've agreed to keep the bindings Python 2 compatible until at least after the 13 release is cut. (https://lists.llvm.org/pipermail/lldb-dev/2020-August/016388.html)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95712