Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/80...@github.com>
sam-mccall wrote: TL;DR: sounds like I should revert the removals/deps in clangd, and let you decide how to structure clang-tidy's copy of the code? `tidy::utils` as a dep has a conceptual layering problem, and a few practical ones: it brings in a pile of dependencies that aren't otherwise in the no-tidy build (effect: much longer build times), and it sets the wrong expectations around performance: that meeting the batch-latency requirements of clang-tidy is sufficient, rather than the interactive-latency requirements of clangd. (Not a theoretical concern: clangd's *deliberate* deps on clang-tidy have had multiple unacceptable performance regressions in the past, which is the cost of taking the dep, but not one that needs paying here). > This were main reason, why it were avoided, as there were no unit tests for > this code on clangd side. This wasn't a library in its own right on the clangd side. If it's a library with multiple clients, it needs tests so problems with the library can be distinguished from problems with the clients. (I think it would have been nice to have fine-grained unittests anyway, but we didn't). > Why is it such a big deal that a revert needs to be considered? It's not a big deal, but I thought the most likely fix was to move this into a separate library with tests, I wasn't sure if anyone wanted to take that on in a hurry, and wanted to get back to a good state. If tidy owners are happier with just cloning the code, I can revert just the changes to clangd. I would suggest moving the code out out tidy/utils and into the check if that's the only place it's going to be tested, but that's up to you. https://github.com/llvm/llvm-project/pull/80541 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
