[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 cfe-commits at lists.llvm.org
Fri May 29 08:07:20 PDT 2020


curdeius added a comment.

That looks nicer indeed. Thanks.
Just some minor nitty-gritty comments.



================
Comment at: clang/docs/tools/generate_formatted_state.py:52
+        path = os.path.relpath(root, LLVM_DIR)
+        if "/test/" in path:
+            continue
----------------
MyDeveloperDay wrote:
> curdeius wrote:
> > That doesn't work on Windows because of slashes. You doesn't skip `unittests` (present at least in clang and llvm).
> So unit tests is something that I I think needs to be clang-formatted, this is because often we are actively editing in there, (I use format on save) and so having clean tests is super important
> 
> The tests directories normally have 100's of small snippets of code and some may even be testing unformatted code deliberately, these files are often made once and not continuously edited, (whilst it would be good to have them clean, I wanted to give ourselves a fighting chance!)
> 
> Point taken about Windows, whilst I develop myself on Windows I use cygwin which is why it probably worked.
OK, I agree for unittests. But then one could argue that the same should apply for test, nope?


================
Comment at: clang/docs/tools/generate_formatted_state.py:30
+        .good {{ background-color: #2CCCFF }}
+        </style>
+
----------------
Nit: wrong indentation.


================
Comment at: clang/docs/tools/generate_formatted_state.py:74
+            continue
+        head, tail = os.path.split(root)
+        while head:
----------------
Both `tail` and `_tail` seem unused. You can change to `_`.


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

https://reviews.llvm.org/D80627





More information about the cfe-commits mailing list