ymandel marked an inline comment as done. ymandel added inline comments.
================ Comment at: clang/include/clang/Tooling/Transformer/Transformer.h:28 +// also update their code to reference `TransformerTool`. +#include "clang/Tooling/Transformer/TransformerTool.h" +// Temporary alias to assist clients in migrating to new class name. ---------------- gribozavr wrote: > I'm not a fan of umbrella headers... especially if they don't cover > everything. > > I think "include what you use" is a better rule. > > The rest of the patch makes sense to me, just not this file. (OK to keep it > temporarily if we need it to help clients migrate, of course.) Thanks, sounds good. In that case, might it make more sense to just split off RewriteRule and not rename Transformer -> TransformerTool (along w/ the correpsonding header change)? My primary motivation for renaming the class was to allow Transformer to be the name for the umbrella. Since Transformer.h will have to include RewriteRule.h anyhow, there won't be any need for a forwarding header while we update users. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68795/new/ https://reviews.llvm.org/D68795 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
