[PATCH] D146655: [clang-tidy] Ignore DISABLED_ in test suite name in google-avoid-underscore-in-googletest-name

Carlos Galvez via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Mar 23 00:18:11 PDT 2023


carlosgalvezp added inline comments.


================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:237
 
+- Fixed an issue in :doc:`google-avoid-underscore-in-googletest-name
+  <clang-tidy/checks/google/avoid-underscore-in-googletest-name>` when using
----------------
Eugene.Zelenko wrote:
> Please keep alphabetical order (by check name) in this section.
I was planning to do that but noticed that the alphabetical order is already broken. It seems to be a source of friction and there's no official documentation that states it should be done like that, so I can understand if it gets broken often. Do you know if this is documented somewhere? If not, do we see value in keeping this convention? I suppose now we would need an NFC patch to fix the order again, causing churn.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp:90
 // Underscores are allowed to disable a test with the DISABLED_ prefix.
-// https://github.com/google/googletest/blob/master/googletest/docs/faq.md#why-should-test-suite-names-and-test-names-not-contain-underscore
+// https://google.github.io/googletest/faq.html#why-should-test-suite-names-and-test-names-not-contain-underscore
 TEST(TestCaseName, TestName) {}
----------------
LegalizeAdulthood wrote:
> PiotrZSL wrote:
> > Would be good if this link would exist also in documentation for this check.
> My experience has been that these external links move around with gtest and are a source of churn.
Good point, I can add on a separate NFC patch :) I believe if we use the github.io (instead of github.com) the links can be a bit more stable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146655



More information about the cfe-commits mailing list