VitaNuo marked an inline comment as done. VitaNuo added inline comments.
================ Comment at: clang-tools-extra/clangd/IncludeCleaner.h:87 +std::optional<include_cleaner::Header> +firstMatchedProvider(const include_cleaner::Includes &Includes, + llvm::ArrayRef<include_cleaner::Header> Providers); ---------------- kadircet wrote: > VitaNuo wrote: > > kadircet wrote: > > > can you also move tests related to this functionality from HoverTests > > > into IncludeCleanerTests ? > > Not really, not sure what you mean. > > > > This function receives a ranked list of providers that should be a result > > of include_cleaner analysis. > > > > If I just test the function based on a fake list of providers, the test > > will be trivial. The current hover test seems to provide more value than > > that. > > If I just test the function based on a fake list of providers, the test > > will be trivial. > > but that's the "unit" of logic we're trying to test, no? Ok, I've extracted a test. Hopefully that's what you've meant. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147044/new/ https://reviews.llvm.org/D147044 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
