aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land.
LGTM! ================ Comment at: clang-tools-extra/test/clang-tidy/Inputs/Headers/stdio.h:1 +#ifndef _STDIO_H_ +#define _STDIO_H_ ---------------- stephanemoore wrote: > stephanemoore wrote: > > I noticed that some of the other example headers don't have `#ifdef` guards > > at all. I decided to still include them—perhaps mostly for my own sanity. I > > figured that it wasn't necessary to use the full > > `LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_INPUTS_HEADER_STDIO_H` for this > > header that is mimicking a system header so I went with the shorter > > `_STDIO_H_` which I think better represents what actual stdio.h headers > > would have. Let me know if you think something else makes more sense. > Do I need the licensing preamble on this test input header? Yes, please add it. ================ Comment at: clang-tools-extra/test/clang-tidy/Inputs/Headers/stdio.h:1-9 +#ifndef _STDIO_H_ +#define _STDIO_H_ + +// A header intended to contain C standard input and output library +// declarations. + +int printf(const char *, ...); ---------------- aaron.ballman wrote: > stephanemoore wrote: > > stephanemoore wrote: > > > I noticed that some of the other example headers don't have `#ifdef` > > > guards at all. I decided to still include them—perhaps mostly for my own > > > sanity. I figured that it wasn't necessary to use the full > > > `LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_INPUTS_HEADER_STDIO_H` for this > > > header that is mimicking a system header so I went with the shorter > > > `_STDIO_H_` which I think better represents what actual stdio.h headers > > > would have. Let me know if you think something else makes more sense. > > Do I need the licensing preamble on this test input header? > Yes, please add it. I don't think it matters all that much for our tests; I'm fine with the way you have it currently. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58095/new/ https://reviews.llvm.org/D58095 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits