ioeric added a comment. In https://reviews.llvm.org/D24192#533200, @omtcyfz wrote:
> In https://reviews.llvm.org/D24192#533198, @ioeric wrote: > > > In https://reviews.llvm.org/D24192#533174, @omtcyfz wrote: > > > > > In https://reviews.llvm.org/D24192#532981, @ioeric wrote: > > > > > > > - It would make the review easier if you could separate the migration > > > > of clang-rename into another patch... > > > > > > > > > Another point is that if I try to separate the migration - what do I do > > > about USREngine? USREngine is basically the core of clang-refactor at the > > > moment and I can't detach it from clang-rename at the same time. > > > > > > I'm not sure why USREngine is the core of clang-refactor. It seems to me > > that USREngine is more closely tied to clang-rename than to clang-refactor. > > At least USREngine is not essential to all refactor tools, and it is more > > like a library that sub-modules can use. > > > It is essential to all of the tools I wrote about in design doc. > > Well, are you proposing to create an "empty" `clang-refactor` binary in one > patch and adding meaningful code in the other? I am not sure if just creating > `clang-refactor/driver/Driver.cpp` with `main`, which doesn't do anything is > a good idea, but if you think it is - I'll do that. It was not trivial to me why USREngine is so important to those tools. You might want to address that in the design doc as well. And given the weight USREngine carries in clang-refactor as you suggested, I think it deserves an more detailed introduction in the design doc. https://reviews.llvm.org/D24192 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
