[PATCH] D80627: [clang-format] Create a python documentation tool to generate a summary of the clang-format status for the whole of the LLVM project

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

@MyDeveloperDay , I know it's a strange request, but could you change (or remove) the background color for 100% case.
I'm partially color-blind and having red and green background in the same table is really hard to distinguish. I guess I'm not alone.
I'd suggest using something like light blue, it doesn't need to stand out anyway.



================
Comment at: clang/docs/tools/generate_formatted_state.py:50
+    totalFilesFail = 0
+    for root, subdirs, files in os.walk(rootdir):
+        path = os.path.relpath(root, LLVM_DIR)
----------------
Unused `subdirs` variable: change to `_`.


================
Comment at: clang/docs/tools/generate_formatted_state.py:52
+        path = os.path.relpath(root, LLVM_DIR)
+        if "/test/" in path:
+            continue
----------------
That doesn't work on Windows because of slashes. You doesn't skip `unittests` (present at least in clang and llvm).


================
Comment at: clang/docs/tools/generate_formatted_state.py:56
+        while head:
+            fileCount = 0
+            filePass = 0
----------------
curdeius wrote:
> You can move it outside the loop.
Here you use camelCase, but in other places you use snake_case (e.g. `file_path`). Please make that consistent.


================
Comment at: clang/docs/tools/generate_formatted_state.py:56-58
+            fileCount = 0
+            filePass = 0
+            fileFail = 0
----------------
You can move it outside the loop.


================
Comment at: clang/docs/tools/generate_formatted_state.py:80
+        if (fileCount > 0):
+            output.write("   * - " + path + "\n")
+            output.write("     - " + str(fileCount) + "\n")
----------------
Writing `path` directly on Windows will put backslashes that are not rendered in rst, so you should either change backslashes to forward slashes (that's what I'd suggest) or double the backslashes. You can just `path.replace('\\', '/')`.


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

https://reviews.llvm.org/D80627