ymandel 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. This is one of the most complex clang-tidy checks. So, if you're looking for a CT starter task, I wouldn't recommend this particular challenge. That said, I think the clang-tidy side will be relatively easy -- CT has a mature config system/API. The harder part (and not CT relevant) is refactoring this code to consume that config. It's not terribly complicated but will require a bunch of changes and probably some design questions. 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
