[PATCH] D89742: [FileCollector] Move interface into FileCollectorBase (NFC)

Jonas Devlieghere via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 19 21:45:37 PDT 2020


JDevlieghere added a comment.

In D89742#2340769 <https://reviews.llvm.org/D89742#2340769>, @dexonsmith wrote:

> In D89742#2340628 <https://reviews.llvm.org/D89742#2340628>, @JDevlieghere wrote:
>
>> In D89742#2340590 <https://reviews.llvm.org/D89742#2340590>, @dexonsmith wrote:
>>
>>> This mostly looks good, I'm just wondering about tests. I hunted around for a `FileCollector` unit test and didn't see one, but it seems like it might be a good fit, especially if it gets you coverage things otherwise only used in LLDB. WDYT?
>>
>> I think you might have missed `FileCollectorTest.cpp`. I guess I could add some test for just the base class, but I'm not sure if that would add much value.
>
> Weird, `git grep` seems to be returning different results now :). Agreed, it seems like enough.
>
> This LGTM now with the fix for `override`.

Been there... Thanks for the review!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89742/new/

https://reviews.llvm.org/D89742



More information about the llvm-commits mailing list