omtcyfz added a comment. In https://reviews.llvm.org/D24192#533220, @ioeric wrote:
> 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 :) Yes, you are absolutely right. This patch is just a mess, because I tried to push everything at once while most of the stuff is not related to clang-refactor part at all. I pushed 3 very basic cleanup patches (namely https://reviews.llvm.org/rL280638, https://reviews.llvm.org/rL280639 and https://reviews.llvm.org/rL280640), which don't really need reviewing (and also for the sake of development speed). And I also kept clang-rename improvements separate in https://reviews.llvm.org/D24224, which is up for review. I'll update this patch tomorrow. What I'm about to do after is to either: 1. Make two patches, one of which would basically create "empty" `clang-refactor` and introduce dummy submodule to show how it works (as you proposed), and the other would merge `clang-rename` as a submodule. 2. Make one patch, which both creates `clang-refactor` binary and merges `clang-rename` as a submodule. The first variant seems better to me while I see a couple of issues with that, which makes sticking to the second variant a bit less challenging. We can discuss it tomorr..today and come up with a solution. Thank you very much for your comments, Eric! https://reviews.llvm.org/D24192 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
