[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

2020-05-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM! Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80627/new/ https://reviews.llvm.org/D80627 ___ cfe-commits mailing list

[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

2020-05-29 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/docs/tools/generate_formatted_state.py:52 +path = os.path.relpath(root, LLVM_DIR) +if "/test/" in path: +continue curdeius wrote: > MyDeveloperDay wrote: > > curdeius wrote: > >

[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

2020-05-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/docs/tools/generate_formatted_state.py:54 + +table_row = """ * - {path} + - {count} Another nit: I prefer writing `"""\` as it nicely aligns with subsequent lines. CHANGES SINCE LAST ACTION

[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

2020-05-29 Thread Marek Kurdej via Phabricator via cfe-commits
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

[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

2020-05-29 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. Would something like this be easier to read? F12027905: image.png Comment at: clang/docs/tools/generate_formatted_state.py:52 +path =

[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

2020-05-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. @MyDeveloperDay, I've played around with the script, you can take as much as you judge useful from here: https://github.com/mkurdej/llvm-project/tree/arcpatch-D80627. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80627/new/ https://reviews.llvm.org/D80627

[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

2020-05-29 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D80627#2062332 , @curdeius wrote: > @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

[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

2020-05-29 Thread 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

[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

2020-05-28 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius requested changes to this revision. curdeius added a comment. This revision now requires changes to proceed. First of all, very nice idea. :) Second, could you please make sure that the script is compatible with Python 3? Also, in order to clean up a bit the generation of the RST (to

[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

2020-05-28 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D80627#2060463 , @JakeMerdichAMD wrote: > Great idea on this! I may borrow this idea and make something similar for > some migrations I'm working on. > > Sanity check: is this going to be run automatically when docs

[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

2020-05-28 Thread Jake Merdich via Phabricator via cfe-commits
JakeMerdichAMD added a comment. Great idea on this! I may borrow this idea and make something similar for some migrations I'm working on. Sanity check: is this going to be run automatically when docs are generated or done offline with results committed? I don't have a preference, either has

[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

2020-05-28 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/docs/tools/generate_formatted_state.py:17 +with open(DOC_FILE, 'wb') as output: +output.write(".. raw:: html\n") +output.write("\n") It will still not work with Python 3, you need to pass `bytes` to

[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

2020-05-28 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I'm adding a summary table which shows LLVM only 44% of all LLVM cpp/h files are clang-formatted. ;-( F12020299: image.png CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80627/new/ https://reviews.llvm.org/D80627

[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

2020-05-28 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay planned changes to this revision. MyDeveloperDay added a comment. I'm running through flake8... almost every line is changing ;-) ...let me update before wasting any more of your time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[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

2020-05-28 Thread Sylvestre Ledru via Phabricator via cfe-commits
sylvestre.ledru added inline comments. Comment at: clang/docs/tools/generate_formatted_state.py:48 +if "/test/" in path: +print root +continue print xx is python 2 please use print(xx) Comment at:

[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

2020-05-28 Thread Sylvestre Ledru via Phabricator via cfe-commits
sylvestre.ledru added a comment. Well done! For Firefox, we decided to go ahead and reformat everything at once. I has been way easier this way. You should run flake8 & autopep8 on your python code. it isn't idomatic Python code (";" are useless on python for example)