[PATCH] D24380: [migrate-tool] Framework for a codebase-dependent migration tool.

[Haojian Wu via cfe-commits]

hokein added a comment.

Sorry for the delay. 
The patch only contains an unittest for `HeaderGenerato`r, which is not quite enough. Should we create a fake migrate-tool binary to illustrate APIs usage?


================
Comment at: migrate-tool/HeaderGenerator.h:22
@@ +21,3 @@
+public:
+  HeaderGenerator(llvm::StringRef HeaderName);
+
----------------
explicit.

================
Comment at: migrate-tool/MigrateTool.cpp:99
@@ +98,3 @@
+  MovedSymbols.push_back(Spec.getOldName());
+  // FIXME: consider source files. Need to determine whether source files send
+  // with ".cpp" or ".cc" etc.
----------------
s/send/end


================
Comment at: migrate-tool/MigrateTool.h:50
@@ +49,3 @@
+  public:
+    enum class MigrateType {
+      Class,
----------------
What is the 'MigrateType' used for? I can't find any usage in the patch.

================
Comment at: migrate-tool/RefactoringManager.h:28
@@ +27,3 @@
+  addIncludesToFiles(const std::set<llvm::StringRef> &Includes,
+                     llvm::ArrayRef<std::string> Files) = 0;
+
----------------
The arguments `llvm::StringRef` and `std::string` look inconsistent to me at  first glance. But currently it is fine, we can modify it if needed in the future.


https://reviews.llvm.org/D24380