klimek added inline comments. ================ Comment at: migrate-tool/BuildManager.h:22 @@ +21,3 @@ +public: + virtual bool addHeaderOnlyLibrary(llvm::StringRef HeaderPath) = 0; + ---------------- I'm not sure the header-only distinction makes sense. I'd start with virtual bool addLibrary(llvm::ArrayRef<std::string> Sources) = 0; or similar.
Also, needs more comments :) Especially what this would mean for different build systems. ================ Comment at: migrate-tool/BuildManager.h:27 @@ +26,3 @@ + + virtual std::string getTargetForFile(llvm::StringRef File) = 0; +}; ---------------- I think we'll want a more precise name, as this might mean different things (target that compiles File vs target that outputs File, etc). What do you want this to do? ================ Comment at: migrate-tool/HeaderBuild.cpp:19 @@ +18,3 @@ + +std::string joinStrings(const std::vector<std::string> &Strings) { + std::ostringstream SS; ---------------- Can't we use join from ADT/StringExtras.h? ================ Comment at: migrate-tool/HeaderBuild.h:19 @@ +18,3 @@ +// This constructs a C++ header containing type aliases. +class Header { +public: ---------------- I'd probably call this HeaderGenerator or something. ================ Comment at: migrate-tool/HeaderBuild.h:29 @@ +28,3 @@ + + std::string generateContent() const; + ---------------- This all needs more comments :) I assume we'll also need to somehow give this a "style" at some point? Or alternatively create dumb data structures, and have a style interface that can create the actual source according to a style from the data we provide? ================ Comment at: migrate-tool/RefactorManager.h:24 @@ +23,3 @@ +// performs some refactoring operations. +class RefactorManager { +public: ---------------- bkramer wrote: > Documentation for the methods? :) I think "RefactoringManager" reads better; otherwise this sounds like you want to refactor the manager :) That said, I think we'll need to split this - getAffectedFiles belongs in its own interface, and we might want to have a common interface with what Kirill and Benjamin are working on. https://reviews.llvm.org/D24380 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits