gribozavr2 added a comment. In D155890#4523243 <https://reviews.llvm.org/D155890#4523243>, @adukeman wrote:
> In D155890#4522266 <https://reviews.llvm.org/D155890#4522266>, @ymandel wrote: > >> In D155890#4521266 <https://reviews.llvm.org/D155890#4521266>, >> @carlosgalvezp wrote: >> >>> This should be a configuration option, we should not hardcore >>> project-specific things in the source code. >> >> I agree, but we already are hardcoding specific types -- I think this is a >> separate (and valid) critique of the design. I'd propose filing an issue on >> the github tracker and we can follow up there. I, for one, would love to >> review such a change but don't have the time to write it. > > Is moving these values to config an appropriate task for somebody like me new > to working on clang-tidy? I'd be happy to merge this and then try the > transition to a config assuming there's some similar examples I can borrow > from elsewhere in the codebase. I think it can be a good starter task for a new engineer on the project. However, don't underestimate this problem, it will require the code to be refactored a little bit. For example, the function `hasOptionalClassName` needs restructuring so that it can accept class names from a list. Not a lot of work, but it isn't mechanically replacing string literals with a variable either. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155890/new/ https://reviews.llvm.org/D155890 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
