[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-25 Thread Kar Epker via Phabricator via cfe-commits
karepker added a comment. In D56424#1370908 , @hokein wrote: > In D56424#1370461 , @karepker wrote: > > > Rebasing against master. > > > > Not sure if re-arc diffing this is necessary, but I hope it doesn't hurt. >

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-25 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. In D56424#1370461 , @karepker wrote: > Rebasing against master. > > Not sure if re-arc diffing this is necessary, but I hope it doesn't hurt. Didn't notice that you don't have commit access, committed in rL352183

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-25 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL352183: [clang-tidy] Add check for underscores in googletest names. (authored by hokein, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D56424

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-24 Thread Kar Epker via Phabricator via cfe-commits
karepker updated this revision to Diff 183437. karepker added a comment. Rebasing against master. Not sure if re-arc diffing this is necessary, but I hope it doesn't hurt. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56424/new/ https://reviews.llv

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-21 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. Sorry for the delay, I was OOO last week. The check looks good. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56424/new/ https://reviews.llvm.org

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-16 Thread Kar Epker via Phabricator via cfe-commits
karepker added a comment. In D56424#1360318 , @MyDeveloperDay wrote: > In D56424#1359227 , @karepker wrote: > > > In D56424#1357484 , > > @MyDeveloperDay wrote: > > > > > I

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-16 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D56424#1359227 , @karepker wrote: > In D56424#1357484 , @MyDeveloperDay > wrote: > > > In D56424#1357481 , @lebedev.ri > > wrote: > > > >

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-15 Thread Kar Epker via Phabricator via cfe-commits
karepker added a comment. In D56424#1357484 , @MyDeveloperDay wrote: > In D56424#1357481 , @lebedev.ri > wrote: > > > In D56424#1357471 , > > @MyDeveloperDay wrote: > > >

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-15 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D56424#1357481 , @lebedev.ri wrote: > In D56424#1357471 , @MyDeveloperDay > wrote: > > > In D56424#1356959 , @karepker > > wrote: > > > >

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-15 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D56424#1357471 , @MyDeveloperDay wrote: > In D56424#1356959 , @karepker wrote: > > > Hi all, ping on this patch. I've addressed all comments to the best of my > > ability. Is there a

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-15 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D56424#1356959 , @karepker wrote: > Hi all, ping on this patch. I've addressed all comments to the best of my > ability. Is there anything outstanding that needs to be changed? Round about this time of a review we norm

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-14 Thread Kar Epker via Phabricator via cfe-commits
karepker added a comment. Hi all, ping on this patch. I've addressed all comments to the best of my ability. Is there anything outstanding that needs to be changed? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56424/new/ https://reviews.llvm.org/D

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-09 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:27 +static bool isGoogletestTestMacro(StringRef MacroName) { + static const llvm::StringSet<> MacroNames = {"TEST", "TEST_F", "TEST_P", +

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:64 + Check->diag(TestCaseNameToken->getLocation(), + "avoid using \"_\" in test case name \"%0\" according to " + "Googletest FAQ")

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:27 +static bool isGoogletestTestMacro(StringRef MacroName) { + static const llvm::StringSet<> MacroNames = {"TEST", "TEST_F", "TEST_P", +

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-08 Thread Kar Epker via Phabricator via cfe-commits
karepker marked 2 inline comments as done and an inline comment as not done. karepker added inline comments. Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:27 +static bool isGoogletestTestMacro(StringRef MacroName) { + static const llvm::StringSet<> Macro

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-08 Thread Kar Epker via Phabricator via cfe-commits
karepker updated this revision to Diff 180727. karepker marked 4 inline comments as done. karepker added a comment. Update release notes documentation to match check documentation more. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56424/new/ https:

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-08 Thread Kar Epker via Phabricator via cfe-commits
karepker added inline comments. Comment at: docs/ReleaseNotes.rst:161 + + Checks that Googletest test and test case names do not contain an underscore, + which is prohibited by the Googletest FAQ. Eugene.Zelenko wrote: > Please synchronize with first statement

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-08 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:161 + + Checks that Googletest test and test case names do not contain an underscore, + which is prohibited by the Googletest FAQ. Please synchronize with first statement in documentation.

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-08 Thread Kar Epker via Phabricator via cfe-commits
karepker added a comment. In D56424#1349336 , @lebedev.ri wrote: > In D56424#1349218 , @karepker wrote: > > > Clean up comments in test file. > > > For reference, what documentation sources did you read when creatin

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-08 Thread Kar Epker via Phabricator via cfe-commits
karepker updated this revision to Diff 180716. karepker marked 15 inline comments as done. karepker added a comment. Address comments. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56424/new/ https://reviews.llvm.org/D56424 Files: clang-tidy/goog

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:27 +static bool isGoogletestTestMacro(StringRef MacroName) { + static const llvm::StringSet<> MacroNames = {"TEST", "TEST_F", "TEST_P", +

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-08 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:46 +auto IdentifierInfo = MacroNameToken.getIdentifierInfo(); +if (!IdentifierInfo) { + return; Please elide braces. Repository: rCTE Cla

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-08 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:23 + +constexpr char kDisabledTestPrefix[] = "DISABLED_"; + Please use `llvm::StringLiteral` Comment at: clang-tidy/google/AvoidUnderscoreIn

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:1 +//===--- AvoidUnderscoreInGoogletestNameCheck.cpp - clang-tidy-===// +// nit: space after tidy and before --- Comment a

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-08 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. Thanks, mostly good to me. just a few nits. Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:72 +TestName.consume_front(kDisabledTestPrefix); +if (TestName.find('_') != std::string::npos) { + Check->diag(TestNameToken->

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

2019-01-07 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D56424#1349218 , @karepker wrote: > Clean up comments in test file. For reference, what documentation sources did you read when creating the review itseft? I thought it was documented that an appropriate category (`[clang-