Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Plus replacing a few std::map<string,...> with llvm::StringMap
That doesn't sound NFC since it changes ordering semantics. It seems to me like it should be done separately from the rest of the patch.
It would be nice to split up the rest of the patch too; besides simplifying review, if you leave the #include removals for a final commit it'll be easier to revert if/where necessary.
LGTM, though I think this should be landed in two commits (one for the headers, one for the tests).
clang-tools-extra/test/clang-tidy/checkers/google-module.cpp | ||
---|---|---|
4 | I feel like these testing changes, while NFC, should be landed in a separate patch as they don't have anything to do with the header file changes, right? |
clang-tools-extra/test/clang-tidy/checkers/google-module.cpp | ||
---|---|---|
4 | You're absolutely right I must've forgot to remove them when I removed the other change |
I feel like these testing changes, while NFC, should be landed in a separate patch as they don't have anything to do with the header file changes, right?