[PATCH] D97121: [clang-tidy] Add a Standalone diagnostics mode to clang-tidy

[Nathan James via Phabricator via llvm-commits]

njames93 marked 6 inline comments as done.
njames93 added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:217
   bool AllowEnablingAnalyzerAlphaCheckers;
+  bool SelfContainedDiags;
 };
----------------
steveire wrote:
> The order of members here is starting to look suspicious, size-wise. Not something for this change though.
Not something I'm prepared to worry about in this patch.


================
Comment at: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp:94
+      : IncludeInserterCheckBase(CheckName, Context,
+                                 utils::IncludeSorter::IS_Google, true) {}
+
----------------
steveire wrote:
> I think the `true` here points to this being a boolean trap. Consider using an enum for the parameter instead.
I don't see a point in defining an enum for this when its only a test case, though a parameter comment should be added.


================
Comment at: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp:285
+#include "path/to/header2.h"
+#include "path/to/header.h"
+
----------------
steveire wrote:
> I still find it really confusing that the "single inserter" mode results in multiple of the same header being added.  Perhaps the names should be along the lines of "duplicating" and "deduplicating" instead of "single" and "multi"?
The name of the test is `InsertMultipleIncludesNoDeduplicate`, Is that not sufficient?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97121