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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to