This is an archive of the discontinued LLVM Phabricator instance.

[NFC][Py Reformat] Reformat python files in the rest of the dirs
ClosedPublic

Authored by thieta on May 17 2023, 8:01 AM.

Details

Summary

This is an ongoing series of commits that are reformatting our
Python code. This catches the last of the python files to
reformat. Since they where so few I bunched them together.

Reformatting is done with black.

If you end up having problems merging this commit because you
have made changes to a python file, the best way to handle that
is to run git checkout --ours <yourfile> and then reformat it
with black.

If you run into any problems, post to discourse about it and
we will try to help.

RFC Thread below:

https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style

Diff Detail

Event Timeline

thieta created this revision.May 17 2023, 8:01 AM
Herald added a reviewer: rafauler. · View Herald Transcript
Herald added a reviewer: Amir. · View Herald Transcript
Herald added a reviewer: maksfb. · View Herald Transcript
Herald added projects: Restricted Project, Restricted Project, Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
thieta requested review of this revision.May 17 2023, 8:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 17 2023, 8:01 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Mordante accepted this revision.May 17 2023, 9:38 AM
Mordante added a subscriber: Mordante.

Thanks for working on this! LGTM!

Note libc++ has more python files like libcxx/utils/generate_header_inclusion_tests.py are these intended to be reformatted too.
If so I really prefer to postpone these files. I have some patches for them and I would prefer to avoid merge conflicts.

Another question is it intended to validate the formatting? For libc++ we validate the formatting of our C++ code in the pre-commit CI. I really think it would be great to do that for these files too. If wanted I can do that too.

This revision is now accepted and ready to land.May 17 2023, 9:38 AM
thieta planned changes to this revision.May 17 2023, 10:08 AM

Ugh. Seems like my find command didn't include the .py files. I will update this diff later.

MatzeB added a comment.EditedMay 17 2023, 10:49 AM

Thanks for working on this! LGTM!

Note libc++ has more python files like libcxx/utils/generate_header_inclusion_tests.py are these intended to be reformatted too.
If so I really prefer to postpone these files. I have some patches for them and I would prefer to avoid merge conflicts.

Another question is it intended to validate the formatting? For libc++ we validate the formatting of our C++ code in the pre-commit CI. I really think it would be great to do that for these files too. If wanted I can do that too.

I don't think postponing for some changes makes sense. In fact I would expect this situation to be somewhat common, there is usually more silent unknown downstream users with custom changes that will also trigger merge conflicts.

The way to deal with this is to also run black on your changes and merging should be easy enough again. Something like copy your changes away, run black on copy, on merge conflict copy changes back to resolve...

thieta updated this revision to Diff 523677.May 19 2023, 12:35 AM

Include all .py files (last diff was missing a lot of them).
rebased

This revision is now accepted and ready to land.May 19 2023, 12:35 AM
Herald added a reviewer: MaskRay. · View Herald Transcript
Herald added projects: Restricted Project, Restricted Project, Restricted Project. · View Herald Transcript
Herald added subscribers: libc-commits, openmp-commits, Restricted Project and 13 others. · View Herald Transcript

This is now updated with all the files. The diff became quite a bit bigger. Let me know if anyone wants me to break something out and put it in a separate diff.

jhenderson accepted this revision.May 23 2023, 12:24 AM

The files I have a vague relationship with, namely the top-level cross-projects-tests lit.local.cfg, and the lld files LGTM (I'm not a fan of the two blank line spacing either side of functions, but it is what it is).

sivachandra accepted this revision.May 23 2023, 12:31 AM
sivachandra added a subscriber: sivachandra.

OK for libc parts.