https://pantsbuild.org/ logo
Join SlackCommunities
Powered by
in the interest of not losing work i'll look at ma...
# general
a
in the interest of not losing work i'll look at making black support 2 spaces, forking it now
f
Read their readme: I would be really surprised if that gained traction for them
a
we are not the psf!
h
Someone already forked this! I’m on phone but this was a year or two ago. Look at their closed issues, @wide-energy-11069 had opened a ticket about adding support for two spaces. Last time I checked, the fork was outdated
f
Not sure where you think you are “loosing work”?
a
so that we don't have to revert the changes
h
I wouldn't recommend reverting this, over an arbitrary formatting decision that some people (myself included) disagree with. I can make a weak argument for why 2 spaces is better than 4 spaces, but it is too weak to drive any decisions.
a
i'm pretty sure the change to black is a one-line change
h
It's as if someone crept into the garden in the night and painted the bike shed a different color. OK, it's not what I wanted, but it's not worth repainting it back...
👌 1
And since Black's whole thing is that it's opinionated, I doubt they'd welcome such a change (although I remain surprised that this is their choice...)
a
ok, but we have our own versions of tools all the time. if it's not a huge change and/or difficulty to maintain the fork, does that change the calculus?
h
Painted it a different color, although at the same time installed an automatic bike lock system 😀
h
Dunno, TBH
a
i'll put up a prototype
h
I will set my IDE to 4 spaces instead of 2, and probably forget all about it within a week.
Up to you, but I'm not sure that's worth the work. Might as well embrace our new formatting overlords.
a
unless 4 space indent is slated to be a requirement for later python versions, or there's further tooling support for 4 spaces instead of 2, i don't think this change is specifically warranted just because this single tool happens to have made a choice which is very easy to revert.
h
Python community is converging around Black, and consequently, four spaces. Pytest, Django, Tox, attrs, SQLAlchemy, Pipenv, Virtualenv, and CPython’s dataclasses module all use it now. Not to mention PEP8 has always called for four spaces That, of course, does not mean we must do what the greater community does, but worth acknowledging the direction of the greater ecosystem
a
that sounds like great content for a discussion involving @enough-analyst-54434, @happy-kitchen-89482, and relevant stakeholders at twitter regarding the recent formatting changes in which we get explicit sign-off from pants contributors by using pants-devel.
h
I think we have a bigger problem, around black's apparent inability to handle triple-quoted strings correctly.
a
that's not super good
h
@happy-kitchen-89482 that is a known issue with docstrings that Pierre is working to fix! They don’t touch docstrings, so the indentation stayed the same
a
should we have fixed the issue before formatting the whole repo?
h
There’s an autoformatter for docstrings that Pierre is going to put a PR to fix
h
A known issue is still an issue... Right now it is preventing me from resolving merge conflicts, as running black requires running pants, which requires the source to not be broken...
Pierre's rebasing instructions don't work for this reason.
Also, that autoformatter only solves the problem for docstrings, not for other uses of triple-quoted strings, which we have many of, e.g., in tests.
h
Hm it’s broken for triple strings in general? That would indeed be a major problem, although would be surprised given how widely used the tool is
h
Yep, as far as I can tell.
a
it might be because we had an entire large codebase formatted with two spaces and moved it to 4 in a single pass, not sure if that's typical
h
@aloof-angle-91616 Django did the same thing
a
especially if it doesn't touch triple quoted strings at all, which were also written with 2 spaces in mind
did django have a discussion about it on their mailing list beforehand?
3 Views
Open in Slack
PreviousNext
Powered by