ioeric added a comment. In https://reviews.llvm.org/D24192#533082, @omtcyfz wrote:
> In https://reviews.llvm.org/D24192#532981, @ioeric wrote: > > > - You mentioned a design doc in the summary; maybe also include a link to > > it? > > > Done. > > > - It would make the review easier if you could separate the migration of > > clang-rename into another patch...I think clang-refactor is really the > > interesting part here. And maybe also add a small dummy sub-module as an > > example demonstrating how to add a new sub-module, which I believe will be > > helpful for future developers. (See also tool-template > > <https://github.com/llvm-mirror/clang-tools-extra/tree/master/tool-template> > > which exists for the same purpose.) > > > I am not against it, but I do not think that an empty tool, which just > introduces the binary without any code would be reasonable. This is basically > why I wanted it to be all-in-place. > > The whole changelist doesn't introduce anything really new, so I assume it > might be fine. But if you are not okay with it I can split. Don't worry about a patch being small. Reviewers like small patches :) The reason that I suggested a dummy sub-tool is to lower the bar for developers, especially those who have never developed a clang tool, so that they can easily figure out how to set up a clang-refactor sub-tool. For a "swiss-army knife" tool like clang-refactor, I would worry more about the extendibility of the infrastructure instead of functionality at this stage. At least, I would expect it to be much easier to create a refactor tool under clang-refactor than creating a standalone clang tool. For example, it would be really nice if there is a tool base that a sub-module can derive from so that I don't have to go through the whole process of creating a clang tool and setting up unit/lit test environment :) https://reviews.llvm.org/D24192 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
