This is an archive of the discontinued LLVM Phabricator instance.

[docs] Add Python coding standard to documentation
ClosedPublic

Authored by thieta on Feb 12 2023, 12:01 PM.

Diff Detail

Unit TestsFailed

Event Timeline

thieta created this revision.Feb 12 2023, 12:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 12 2023, 12:01 PM
thieta requested review of this revision.Feb 12 2023, 12:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 12 2023, 12:01 PM
MatzeB added a subscriber: MatzeB.Feb 13 2023, 11:33 AM
MatzeB added inline comments.
llvm/docs/CodingStandards.rst
111–119

We already specify the python version in GettingStarted.rst repeating it here increases the chances that it will get missed and be outdated on the next version bump. So maybe it would be better to not mention any version numbers for python and black here (and possibly link to GettingStarted.rst)?

jhenderson added inline comments.Feb 14 2023, 12:30 AM
llvm/docs/CodingStandards.rst
111–119

I think it's okay to mention the version number of black here, since it is the only place black will be mentioned in the docs. I agree on the python verison though.

JDevlieghere added inline comments.
llvm/docs/CodingStandards.rst
111–119

Agreed with James

117

Is "recommend" strong enough? I was looking for similar wording related to clang-format but it doesn't seem to be part of the coding standards? How about something like:

For consistency and to limit churn, code should be automatically formatted with the black utility.

thieta updated this revision to Diff 521624.May 12 2023, 5:58 AM

Hi! After that nudge at EuroLLVM, I picked this one up and hopefully addressed your comments!

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

LGTM, but best wait for others too.

This revision is now accepted and ready to land.May 12 2023, 6:24 AM
This revision was landed with ongoing or failed builds.May 15 2023, 1:58 AM
This revision was automatically updated to reflect the committed changes.