[Lldb-commits] [PATCH] D139945: [lldb] Add scripted process launch/attach option to {, platform }process commands

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM modulo inline comments.



================
Comment at: lldb/include/lldb/Target/Process.h:235-239
+  std::string m_scripted_process_class_name; // The name of the class that will
+                                             // manage a scripted process.
+  StructuredData::DictionarySP
+      m_scripted_process_dictionary_sp; // A dictionary that holds key/value
+                                        // pairs passed to the scripted process.
----------------
I know you're just doing this for consistency but please use Doxygen style comments `///` on the line before. Bonus points if you fix the ones above as well.


================
Comment at: lldb/source/Commands/CommandOptionsProcessAttach.h:17
+
+// CommandOptionsProcessAttach
+
----------------
Let's stop cargo culting these frankly useless comments.


================
Comment at: lldb/source/Commands/CommandOptionsProcessAttach.h:40
+
+  // Instance variables to hold the values for command options.
+
----------------
Remove


================
Comment at: lldb/source/Commands/CommandOptionsProcessAttach.h:43
+  lldb_private::ProcessAttachInfo attach_info;
+}; // CommandOptionsProcessAttach
+
----------------



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

https://reviews.llvm.org/D139945