carlosgalvezp added a comment.

Looks great, small comments!



================
Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:89
   if (IO.outputting()) {
+    std::vector<std::pair<StringRef, StringRef>> SortedOptions;
+    SortedOptions.reserve(Options.size());
----------------
Maybe add a one-line comment like `Ensure check options are sorted` to more 
quickly grasp what the block of code below does?


================
Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:90
+    std::vector<std::pair<StringRef, StringRef>> SortedOptions;
+    SortedOptions.reserve(Options.size());
+    for (auto &Key : Options) {
----------------
I believe you can pass this directly to the constructor in the previous line, 
to skip the `reserve()` part.


================
Comment at: clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp:58
+
+// RUN: clang-tidy --checks="-*,readability-identifier-*" --dump-config | grep 
readability-identifier-naming | sort -c
----------------
Use the long name of the option, i.e. `--check`, so it's easier to see what 
it's doing without having to check the manual.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156452

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

Reply via email to