[llvm] [lit] Fix some issues from --per-test-coverage (PR #65242)

Tobias Hieta via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 11 08:52:34 PDT 2023


tru wrote:

> > > (By the way, I have noticed that many python files were formatted with >80 columns, probably the black default of 88. That doesn't fit in the 80-column editor/terminal windows I use for most of LLVM. Is there any good reason not to follow [the LLVM coding standard for source code width](https://llvm.org/docs/CodingStandards.html#source-code-width) in python files? I see no exception there for python, or did I misunderstand something? Black just needs a `-l80` command-line option.)
> > 
> > 
> > This was discussed when we wrote the coding style for python and then we thought it was just better to use black's default to avoid having people need to run black with arguments
> 
> OK, but the LLVM coding standard appears to contradict that approach, as far as I can tell:
> 
> 1. "Write your code to fit within 80 columns."  (From https://llvm.org/docs/CodingStandards.html#source-code-width.)
> 2. "The Python code within the LLVM repository should adhere to the formatting guidelines outlined in [PEP-8](https://peps.python.org/pep-0008/)."  (From https://llvm.org/docs/CodingStandards.html#python-version-and-source-code-formatting).
> 3. "Limit all lines to a maximum of 79 characters." (From https://peps.python.org/pep-0008/#maximum-line-length.)
> 
> The [LLVM coding standard's advice on using black](https://llvm.org/docs/CodingStandards.html#python-version-and-source-code-formatting) is:
> 
> > For consistency and to limit churn, code should be automatically formatted with the [black](https://github.com/psf/black) utility. Black allows changing the formatting rules based on major version. In order to avoid unnecessary churn in the formatting rules we currently use black version 23.x in LLVM.
> 
> Combining all the above, I'm led to believe I should be passing options to black, but it's unclear which ones.
> 
> I'm sorry I didn't participate in the original discussion. If it's too late to change things, can we clarify the LLVM coding standard?
> 
> > (since it won't read a config file).
> 
> See https://black.readthedocs.io/en/stable/usage_and_configuration/the_basics.html#configuration-via-a-file.
> 
> For example, in `llvm/utils/lit/pyproject.toml `, I appended the following:
> 
> ```
> [tool.black]
> line-length = 80
> ```
> 
> And it worked for me.

I think that's pretty new then. We tried this back during the original discussion without luck. But feel free to either update the docs to reflect the reality or post to discourse suggesting a policy change. 

I have no strong feelings either way as long as we don't have to remember to manually change options. A change here would also mean a lot of reformatting which is a bit unnecessary churn. 

https://github.com/llvm/llvm-project/pull/65242


More information about the llvm-commits mailing list