|
Hi Tim,
I didn't mean to imply the original decisions were foolish, in
their context. Reading it again, my language in the email was too
strong - it was directed at myself as much as anyone else, because
I'm also the kind of person who loves getting everything
consistent, often too much, and that wasn't clear. What I meant to
say was that if it turns out that our guidelines are becoming a
problem, we should drop them, or change how we regard them (e.g.
as guidelines for those who want to go the extra mile, not as
rules that will frustrate contributors).
Your work on the Django code base has been tireless and
fantastic, and along with the other fellows I don't know what we'd
do without you, sorry for not making that clear with my comments.
Thanks,
Luke
On 19/04/2019 15:51, Tim Graham wrote:
I'm sorry for establishing some (foolish?)
guidelines that discouraged some people from contributing. I
didn't establish those guidelines merely to be pedantic, but
perhaps I'm too much of a perfectionist at times.
After GitHub allowed mergers to amend pull requests, I often
made cosmetic adjustments myself to eliminate the trivial back
and forth that some have lamented. I never blocked a merge
merely for trivial cosmetic reasons.
On Friday, April 19, 2019 at 3:25:24 AM UTC-4, Luke Plant wrote:
Hi all,
An alternative solution to the problem of nit-picky,
formatting related PR reviews is simple: let's not do
that.
We already have an automated code formatting enforcer,
flake8. Let's simply drop the requirement on fixing
anything that flake8 doesn't pick up. A committer can fix
up style issues if they want to, but shouldn't make anyone
else do it. This would mean most of the stuff on our coding style page
should just be delete, or at least not enforced - by which
I mean almost anything that can't be enforced by a tool
(such as isort, flake8, editors via .editorconfig etc.),
and has no non-local effects. (So consistent naming of
classes/functions *should* be enforced, because that
affects other people's ability to use the code). Large
parts of that page are just duplicating of flake8/isort
rules anyway. Honestly, does it kill us if someone writes
"we" in a code comment? And black couldn't help with some
of these things anyway. Let's just stop being code review
jerks.
I'm kind of ambivalent on black itself. Certainly there
are cases where it makes code less readable (a significant
sin in my book) e.g. lists that are better displayed
vertically, as mentioned already, and there are cases
where it makes your diff larger than it needs to be (e.g.
when it decides a list is now too long and needs to be
re-formatted vertically). If we adopt black we'd have to
live with those annoyances. Alternatively, we can live
with the annoyance that code formatting is not perfectly
consistent and we accept less than 'perfect' PR. But we
should just live with those things:
A foolish consistency
is the hobgoblin
of little minds
And if our consistency requirements are causing problems
for people attempting to contribute, they are foolish and
should be dropped.
My 2 ¢.
Luke
On 18/04/2019 16:03, Jani Tiainen wrote:
Well let me add my two cents here since I
was also in the group in DCEU that talked about the
usage of black.
Personally I don't like to contribute to Django.
And this is why:
Day one: I'll make the fix/patch and create PR
Day two (or four or five depending how busy
reviewers are): I missed a comma or some minor indent
is wrong
Day three: I fix styles
Day four: PR is accepted.
So whole round trip took about a five days (give a
take few usually depending how busy reviewers are).
That gives me a feeling that I'm really wasting my
time and since I can't get all the small bits and
pieces exactly as Django wants in correct place.
And that's because we have slightly different rules
at the work. And some other projects do have different
rules.
So it would be great if some of this pain could be
relieved with a tool. In my short experience with
black (I've been using it for work projects) it does a
pretty decent job.
Like others have said black does some decisions I
don't agree with. But I don't have to. Black does it
for a "greater good". And after a while black actually
vanishes from the flow.
Hi.
I propose that Django starts using 'black' [0] to
auto-format all Python code.
For those unfamiliar with 'black' I recommend reading
the the projects README.
The short version: it aims to reduce bike-shedding and
non value-adding
discussions; saving time reviewing code; and making
the barrier to entry lower
by taking some uncompromissing choices with regards to
formatting. This is
similar to tools such as 'gofmt' for Go and 'prettier'
for _javascript_.
Personally I first got involved contributing to Django
couple of weeks back,
and from anecdotal experience I can testify to how
'formatting of code' creates
a huge barrier for entry. My PR at the time went
multiple times back and forth
tweaking formatting. Before this, I had to research
the style used by exploring
the docs at length and reading at least 10-20
different source – and even those
were not always consistent. At the end of the day I
felt like almost 50% of the
time I used on the patch was not used on actually
solving the issue at hand.
Thinking about code formatting in 2019 is a mental
energy better used for other
things, and it feels unnecessary that core developers
on Django spend their time
"nit-picking" on these things.
I recently led the efforts to make this change where I
work. We have a 200K+
LOC Django code-base with more than 30K commits. Some
key take-aways: it has
drastically changed the way we work with code across
teams, new engineers are
easier on-boarded, PR are more focused on
architectural choices and "naming
things", existing PRs before migration had
surprisingly few conflicts and were
easy to fix, hot code paths are already "blameable"
and it's easy to blame a
line of code and go past the "black-commit", and
lastly the migration went
without any issues or down-time.
I had some really fruitful discussions at DjangoCon
Europe this week on this
very topic, and it seems we are not alone in these
experiences. I would love to
hear from all of you and hope that we can land on
something that will enable
*more* people to easier contribute back to this
project.
I've set up how this _could_ look depending on some
configurables in Black:
* Default config: https://github.com/hermansc/django/pull/1
* Line length kept at 119: https://github.com/hermansc/django/pull/3
* Line length kept at 119, no string normalization:
https://github.com/hermansc/django/pull/2
Please have a look at the Black documentation. It
explains the benefits better
than I possibly could do here.
With kind regards,
Herman Schistad
[0]: https://github.com/ambv/black
--
You received this message because you are subscribed
to the Google Groups "Django developers
(Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving
emails from it, send an email to django-d...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAN%3DnMTx0EE5WfXuccv_e3MBuCxp9u_pAV_ow5MxNST6MptTDBw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
--
Jani Tiainen
Software wizard
Always open for short term jobs or
contracts to work with Django.
--
You received this message because you are subscribed to
the Google Groups "Django developers (Contributions to
Django itself)" group.
To unsubscribe from this group and stop receiving emails
from it, send an email to django-d...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAHn91ofkXhofPR_3bjSg2Swei2SyQ_axo6Ymy2dSuKg8G878GA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
You received this message because you are subscribed to the Google
Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it,
send an email to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/06cf99d0-0521-4441-9252-86ef7f779039%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/5f47f119-c76c-ab90-78af-dc38a7df7700%40cantab.net.
For more options, visit https://groups.google.com/d/optout.
|
