-
Notifications
You must be signed in to change notification settings - Fork 10.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[docs] Add more details about Python formatting #66141
Conversation
Describe the darker utility. Make it clear that black/darker's default rules should be used. See ongoing discussion at <https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257>.
llvm/docs/CodingStandards.rst
Outdated
<https://pypi.org/project/darker/>`_ utility with its default rules to format | ||
any changed Python code. Doing so should ensure the patch will pass the Python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not have a specific suggestion, but could we make it clearer that darker
is something that calls black
rather than a different formatter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hah, yeah I was just going to post the same suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. Done.
Also add some quick examples.
llvm/docs/CodingStandards.rst
Outdated
|
||
.. code-block:: bash | ||
|
||
$ pip install black darker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should pin versions here, maybe use a requirements.txt or something? We have it stated above that we recommend version 23.x for now, and the point of this is that black only guarantees a stable format within the major versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To keep the example simple, does the following seem reasonable?
$ pip install black=='23.*' darker
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
llvm/docs/CodingStandards.rst
Outdated
|
||
When contributing a patch unrelated to formatting, you should format only the | ||
Python code that the patch modifies. For this purpose, use the `darker | ||
<https://pypi.org/project/darker/>`_ utility to call black with its default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably say something like "darker utility which runs default black rules over only the changed patch"..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Let me know if the wording I chose doesn't satisfy the concern.
Wordsmith. Specify black version in example `pip install` command. In the example, use `HEAD` instead of `@` as more git users probably know the former. In the example, drop `..:WORKTREE:` as it's hard to remember and, fortunately, redundant. Leave that part to the darker documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Describe the darker utility. Make it clear that black/darker's default rules should be used. Add some examples. See ongoing discussion at <https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257>.
Describe the darker utility. Make it clear that black/darker's default rules should be used.
See ongoing discussion at
https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257.