[PATCH] D24192: [clang-refactor] introducing clang-refactor

[Eric Liu via cfe-commits]

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