Details
Diff Detail
- Build Status
Buildable 4240 Build 4240: arc lint + arc unit
Event Timeline
LGTM.
change-namespace/tool/ClangChangeNamespace.cpp | ||
---|---|---|
78 | Maybe consider create a command-line option for it? It will make the tool more pluggable IMO. |
change-namespace/tool/ClangChangeNamespace.cpp | ||
---|---|---|
82 | Instead std::vector<std::string>, maybe std::vector<llvm::Regex> is better, with that we don't need to do transform stuff in ChangeNamespaceTool. | |
test/change-namespace/Inputs/white-list.txt | ||
1 ↗ | (On Diff #89637) | As this is only one-line file, I'd create this file in whitelist lint test like echo XX > whitelist.txt to avoid adding a new file in test. |
unittests/change-namespace/ChangeNamespaceTests.cpp | ||
42–43 | /*WhiteListedSymbolPatterns*/{} is enough. |
- Addressed comments.
change-namespace/tool/ClangChangeNamespace.cpp | ||
---|---|---|
82 | I'd like ChangeNamespaceTool to own the Regex vector and make sure it is not shared since Regex is not thread safe. |
Maybe consider create a command-line option for it? It will make the tool more pluggable IMO.