[PATCH] D69420: [clang][clang-scan-deps] Add support for extracting full module dependencies.

[Kousik Kumar via Phabricator via cfe-commits]

kousikk added a comment.

A few nit comments



================
Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h:34
+/// The format that is output by the dependency scanner.
+enum class ScanningFormat {
+  /// This is the Makefile compatible dep format.
----------------
"ScanningOutputFormat" or "DependencyOutputFormat" ?


================
Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h:35
+enum class ScanningFormat {
+  /// This is the Makefile compatible dep format.
+  Make,
----------------
IIUC, this format would NOT include module dependencies right?

If so, can you explicitly state that in the comment?


================
Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp:94
+
+    void printDependencies(std::string &S, StringRef MainFile) {
+      // Sort the modules by name to get a deterministic order.
----------------
Should output arguments be at the end?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69420