[libc-commits] [PATCH] D79192: [libc] Add integration tests.

Fri May 29 15:52:42 PDT 2020

PaulkaToast marked an inline comment as done.
PaulkaToast added inline comments.

================
Comment at: libc/utils/HdrGen/PublicAPICommand.h:42

+class APIReader {
+public:
----------------
sivachandra wrote:
> PaulkaToast wrote:
> > sivachandra wrote:
> > > PaulkaToast wrote:
> > > > sivachandra wrote:
> > > > > I see only one specialization of this class. In which case, why is this need?
> > > > Right now the APIGenerator is defined in the .cpp file, so this is used to expose the parts we want. The alternative is to expose the APIGenerator class in the header, but that'd be a larger refactor. It can be done though, which would you prefer?
> > > Yes. Refactor it separately. It need not really be a refactor. I am OK even if you just make API generator public. As it stands in this patch, the specialization and the base class have no meaning as `makeAPIReader` is really not a factory method.
> > > 
> > > Splitting the CMake parts and the hdrgen parts into two patches makes it easier for me to review as well.
> > Okay, this is now pulled into a desperate patch: D80832.
> > When that patch gets merged in I'll pull it in here and update the diff.
> [This is very likely a typo but funny.]
> No, my intention was not to make you feel desperate.
I'm so sorry that was a typo, meant to write "separate". 😅 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79192