Proposal to format Django using black

classic Classic list List threaded Threaded
107 messages Options
1234 ... 6
Reply | Threaded
Open this post in threaded view
|

Proposal to format Django using black

Herman Schistad
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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|

Re: Proposal to format Django using black

Ian Foote
I'm in favour. Not needing to think much about code style when writing code or when reviewing is very nice.

Ian

On Sat, 13 Apr 2019 at 13:52, Herman S <[hidden email]> wrote:
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 [hidden email].
To post to this group, send email to [hidden email].
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.

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/CAFv-zf%2BAo57rc4k9%2B0Ndu%3DEVprv5JLGu0tM2ihdwWcddCr%3DcXg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Proposal to format Django using black

Florian Apolloner
In reply to this post by Herman Schistad
As expressed at Djangocon Europe, I am hugely in favor for adopting black.

If we choose to do this there are a few things to consider:

 * Line-length, we probably want to stay at 119 I guess
 * String normalization, I'd be in favor of following black defaults here, but I am open to be convinced otherwise
 * We will need to adjust the isort configuration ( https://github.com/ambv/black/blob/master/README.md#how-black-wraps-lines )

Cheers,
Florian

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/698d8e49-0deb-437c-b6ce-a321fe3988e3%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Proposal to format Django using black

Markus Holtermann
👍

/Markus

On Sat, Apr 13, 2019, at 6:08 PM, Florian Apolloner wrote:

> As expressed at Djangocon Europe, I am hugely in favor for adopting black.
>
> If we choose to do this there are a few things to consider:
>
>  * Line-length, we probably want to stay at 119 I guess
>  * String normalization, I'd be in favor of following black defaults
> here, but I am open to be convinced otherwise
>  * We will need to adjust the isort configuration (
> https://github.com/ambv/black/blob/master/README.md#how-black-wraps-lines )
>
> Cheers,
> Florian
>
>  --
>  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 [hidden email].
>  To post to this group, send email to
> [hidden email].
>  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/698d8e49-0deb-437c-b6ce-a321fe3988e3%40googlegroups.com <https://groups.google.com/d/msgid/django-developers/698d8e49-0deb-437c-b6ce-a321fe3988e3%40googlegroups.com?utm_medium=email&utm_source=footer>.
>  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 [hidden email].
To post to this group, send email to [hidden email].
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/8e323d0c-b077-4bbd-8384-19cf6022a445%40www.fastmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Proposal to format Django using black

Jon Dufresne
In reply to this post by Herman Schistad
Big +1

I've used black in quite a few projects now and found it has greatly reduced the mental energy involved in code formatting while aligning all contributors expectations.

I'm in favor of adding no configuration. That is, use black's default line length and string delimiters. IMO, the less configuration the better. The more the Django code style aligns with the larger Python community, the better and easier it is few new contributors. But these secondary choices are less important to me than the improvement of adopting a tool to auto-format code.

On Sat, Apr 13, 2019 at 8:35 AM Herman S <[hidden email]> wrote:
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 [hidden email].
To post to this group, send email to [hidden email].
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.

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/CADhq2b4K6ox%3DZ2YPqF%2BWpxGAFkdF1gjViudJ9QhkVn7UKjy%3DRA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Proposal to format Django using black

Tobias McNulty
In reply to this post by Florian Apolloner
On Sat, Apr 13, 2019 at 12:07 PM Florian Apolloner <[hidden email]> wrote:
As expressed at Djangocon Europe, I am hugely in favor for adopting black.

A quick glance suggests this would result in ~20k lines changed (~27% of non-whitespace, non-comment code lines) in django/ and ~58k lines changed (~41%) in tests/ (with `--line-length=119`). After the initial pass to fix everything `black -l 119 tests/ django/` takes only 2.3 seconds to run (!), though I'm sure my OS had all the files cached at that point.

Without normalizing strings, the number of changed lines comes down to ~14k (~19%) for django/ and ~23k (~16%) for tests/ (with `--skip-string-normalization --line-length=119`).

I'm sure some people will see this as "a ton" and others "only that much?" :-)

(My line counts are imperfect and approximate, and you'll likely get somewhat different numbers if you try this yourself.)

If we choose to do this there are a few things to consider:

 * Line-length, we probably want to stay at 119 I guess

Perhaps we could compromise and choose something in the middle, for the reasons noted in the docs: https://black.readthedocs.io/en/stable/the_black_code_style.html#line-length

We should also consider adding max-doc-length for flake8 to our setup.cfg, and/or forego the shorter requirement for comments/docstrings if we shorten the maximum line length for code.

This section in the docs will need updating if we hand this off to a 3rd-party formatter (in particular the sentence, "Don’t limit lines of code to 79 characters if it means the code looks significantly uglier or is harder to read." since we'll no longer have the option of shortening lines to 79 characters arbitrarily).

 * String normalization, I'd be in favor of following black defaults here, but I am open to be convinced otherwise

It'd be nice to minimize the diff, but I'd be fine either way. If we're going to do it eventually, better to do so all at once.
 
 * We will need to adjust the isort configuration ( https://github.com/ambv/black/blob/master/README.md#how-black-wraps-lines )

+1

Overall, the main downside I see is the large, fairly useless changeset this would create and the impact it would have, such as reduced utility of `git blame` for some portion of the code for the foreseeable future and interruption to all in-progress PRs. Problems that can be worked around, sure, but it would be an interruption to ongoing work nonetheless.

One question: Would we want to update some/all code samples in the docs with the same code style? I imagine that could be a fairly time-intensive endeavor, unless someone's already found a way to automate the process. In particular, Black's format for chaining method calls differs from Django's preferred style. Applying this standard to the docs without discretion may lead to less readable docs, IMO...on the other hand, it's odd to have code samples that don't pass our own tests...

In any case, while this sounds like a non-trivial project with some questions to work through yet, I too like not having to worry about code formatting and don't have any real objections to this.

Cheers,


Tobias McNulty
Chief Executive Officer

[hidden email]
www.caktusgroup.com

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/CAMGFDKQo_7aru%3DGJWQGj8g1Fv69iq2H3Wp6%3D0%2Bdp2NKTaHh4OA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Proposal to format Django using black

Adam Johnson-2
In reply to this post by Jon Dufresne
+1

I’d like to see gradual roll out (one module at a time? One rule at a time?) to spread the work of reformatting all the in-flight patches.

On Sat, 13 Apr 2019 at 18:45, Jon Dufresne <[hidden email]> wrote:
Big +1

I've used black in quite a few projects now and found it has greatly reduced the mental energy involved in code formatting while aligning all contributors expectations.

I'm in favor of adding no configuration. That is, use black's default line length and string delimiters. IMO, the less configuration the better. The more the Django code style aligns with the larger Python community, the better and easier it is few new contributors. But these secondary choices are less important to me than the improvement of adopting a tool to auto-format code.

On Sat, Apr 13, 2019 at 8:35 AM Herman S <[hidden email]> wrote:
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 [hidden email].
To post to this group, send email to [hidden email].
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.

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/CADhq2b4K6ox%3DZ2YPqF%2BWpxGAFkdF1gjViudJ9QhkVn7UKjy%3DRA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
--
Adam

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/CAMyDDM0OUCQVU%3D%2B6yJBrA%2B15NUuhHnp2TyWyoL10JJyggVrH0A%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Proposal to format Django using black

Tobias McNulty
In reply to this post by Herman Schistad
On Sat, Apr 13, 2019 at 11:35 AM Herman S <[hidden email]> wrote:
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

Apologies, somehow I missed these links on the first read. Go here for an overview of lines changed, don't use my numbers. :-)

Tobias

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/CAMGFDKQo2W430rqH4oZ7D89bJ4ix-xmxYx_WWYzAajMyZ493Jg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Proposal to format Django using black

Berker Peksağ
In reply to this post by Tobias McNulty
On Sat, Apr 13, 2019 at 9:19 PM Tobias McNulty <[hidden email]> wrote:
> Overall, the main downside I see is the large, fairly useless changeset this would create and the impact it would have, such as reduced utility of `git blame` for some portion of the code for the foreseeable future and interruption to all in-progress PRs. Problems that can be worked around, sure, but it would be an interruption to ongoing work nonetheless.

I came here to say exactly this. Django's codebase is already pretty
consistent with its own style and I think having a usable "git blame"
is much more important than starting to use a new code formatter. IMO,
with a codebase as big as Django, trying to adapt a different style
guide would only cause unnecessary code churn.

The following style is much more readable (and it also makes future
diffs less noisy since we can add new formats without having reformat
code and comments) than black-formatted code:

TIME_INPUT_FORMATS = [
    '%H:%M:%S', # '14:30:59'
    '%H:%M:%S.%f', # '14:30:59.000200'
    '%H:%M', # '14:30'
]

Reformatted by black:

TIME_INPUT_FORMATS = ['%H:%M:%S', '%H:%M:%S.%f', '%H:%M'] # '14:30:59'
# '14:30:59.000200' # '14:30'

(Taken from https://github.com/hermansc/django/pull/2/files#diff-ba8335f5987fcd81d41c28cd1879a9bfL370)

(Also, thank you for creating PRs with different black configurations, Herman.)

--Berker

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/CAF4280%2BB9k0Nymm4Oq%3DxnGe0gnZCzx2cDs3-esEBxDp2W9P-bg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Proposal to format Django using black

Florian Apolloner
Hi,

On Saturday, April 13, 2019 at 9:41:38 PM UTC+2, Berker Peksağ wrote:
I came here to say exactly this. Django's codebase is already pretty
consistent with its own style and I think having a usable "git blame"
is much more important than starting to use a new code formatter.

I am not sure I agree with this argument. We already have had big code changes which make git blame useless (if you are looking for a commit from like 8 years ago or so). All we are doing to do is adding one commit more to that list (and the tooling to work around one such a commit is good, ie git-hyper-blame, git gui blame etc etc).
 
with a codebase as big as Django, trying to adapt a different style
guide would only cause unnecessary code churn.

Maybe, but I think the benefits outweigh the costs -- and I also do not think that it is unnecessary. Our goal has always been to make contributing easier, well nowadays black is in the position to do just that.

The following style is much more readable (and it also makes future
diffs less noisy since we can add new formats without having reformat
code and comments) than black-formatted code:

Yes, the formats (same for settings files) are imo certainly files we could exclude from black. That said those files are highly specific and hardly representative for python code.

Cheers,
Florian

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/7eb37396-04a0-41a7-960f-cbcbd4ac0001%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Proposal to format Django using black

Florian Apolloner
In reply to this post by Adam Johnson-2
On Saturday, April 13, 2019 at 8:23:13 PM UTC+2, Adam Johnson wrote:
I’d like to see gradual roll out (one module at a time? One rule at a time?) to spread the work of reformatting all the in-flight patches.

My thoughts initially. But then I realized that it would be easier for tooling if there were just one commit to exclude (also easier for developers to remember that one) and commits that get merged while we are converting module by module would be very careful over which files to run 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 [hidden email].
To post to this group, send email to [hidden email].
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/5bd7f89e-f2d0-4a30-b014-3b933541dc8e%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Proposal to format Django using black

James Bennett
In reply to this post by Florian Apolloner
On Sat, Apr 13, 2019 at 1:18 PM Florian Apolloner <[hidden email]> wrote:
Maybe, but I think the benefits outweigh the costs -- and I also do not think that it is unnecessary. Our goal has always been to make contributing easier, well nowadays black is in the position to do just that.

I feel like Black is useful for projects that don't already have and enforce a style guide. But we already have and enforce a style guide, and some parts of it are things Black can't auto-enforce. So I'm not sure I see what benefit we'd get out of Black, and the downside is a lot of churn. 

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/CAL13Cg9UNSujA3qCCp4JcvtA5uZRduGOOEKCHp4OCNjCbbH2Zg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Proposal to format Django using black

Carlton Gibson-3
We spend a lot of time spotting small formatting errors and then asking for those to be fixed and then waiting for an update. This wastes reviewer time and slows down the feedback cycle. Many pull requests drag out because of it. 

For this reason I would be 100% behind adopting black, and applying that in a single commit. 

My only reservation is about the history. I don’t know the tools Florian mentioned but I wonder if there is a clever git blame usage that would allow bringing over a single commit change of this sort?

Even if we can’t solve that I’d still be +1 for the added ease to contributors and the speed benefits to review efforts. 

C.

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/CAJwKpyQxxCKfniqmt5qQXG0Nz1oeB4rzhWpT4e_fWTPMDsf-zA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Proposal to format Django using black

Berker Peksağ
On Sun, Apr 14, 2019 at 1:34 AM Carlton Gibson <[hidden email]> wrote:
>
> We spend a lot of time spotting small formatting errors and then asking for those to be fixed and then waiting for an update. This wastes reviewer time and slows down the feedback cycle. Many pull requests drag out because of it.

But you can now do final edits (cosmetic changes, tweak reST markup
and documentation) via GitHub UI, right? I always do it to not bother
contributors with these changes, especially if they are new to the
project.

--Berker

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/CAF4280J%2B6J1BJAto1MHd8mo5X%2Bm4AohSqS2HQr3yNXh1WOUcRQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Proposal to format Django using black

Nick Sarbicki
Just going to say that one of the main frustration points I've had when making a contribution is having to fix small formatting errors (often minor things in docstrings which aren't _always_ consistent).

It produces a lot of inertia and can stop PRs from getting merged for extended periods of time. So from a new contributor angle I think black is an obvious choice.

From a maintainers point of view, git blame is very nice to have and this would add some complexity. But it's easy to see the full version history of a file and, let's be honest, not enough people use git bisect.


On Sun, 14 Apr 2019, 00:04 Berker Peksağ, <[hidden email]> wrote:
On Sun, Apr 14, 2019 at 1:34 AM Carlton Gibson <[hidden email]> wrote:
>
> We spend a lot of time spotting small formatting errors and then asking for those to be fixed and then waiting for an update. This wastes reviewer time and slows down the feedback cycle. Many pull requests drag out because of it.

But you can now do final edits (cosmetic changes, tweak reST markup
and documentation) via GitHub UI, right? I always do it to not bother
contributors with these changes, especially if they are new to the
project.

--Berker

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/CAF4280J%2B6J1BJAto1MHd8mo5X%2Bm4AohSqS2HQr3yNXh1WOUcRQ%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 [hidden email].
To post to this group, send email to [hidden email].
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/CAGuvt92sVXVAuTzLW7sQkpkJEdNuamfeQwjwcn2dvFokZDZ7CQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Proposal to format Django using black

Curtis Maloney-2
So to summarise the discussion so far:

1. automated code formatting will be a great boon - reduce work, lower
barrier for new committers, reduce work for the fellows, etc.

2. there are issues with git history in doing "the great commit".

3. there are issues with black's formatting choices.


So to address 1:

I am entirely +1 for automated code formatting to take the work out of
our hands.

Can such a tool be automated into, say, github in a way that doesn't
create extra commit noise?

To address 2:

I side with those who favour a progressive solution, whereby new code
only has the new tool applied.

That said, there might be cause to suggest a deadline [perhaps a N.0
release?] where all remaining code is "cleaned".

And finally 3:

My perspective on the goal of any code formatting tool is this:

When we as developers approach a piece of code, our goal is the
understand its intent and its implementation of that intent.

In the process of reaching that, we pass through the stages of (a)
identifying the relevant code, (b) understanding the action of the code,
and (c) understanding the intent of the code.

Good code formatting choices will remove or reduce the cognitive load in
(b) and (c).

In my experience with using black [we use it at work], there are
numerous choices (including those demonstrated in this list already)
where it can significantly _increase_ the cognitive load in simply
parsing the code.

As simple as black can make the job of code formatting, I feel I'd
rather see a different tool that retained the benefits of "trivial code
reformatting", but still allowed us to retain some of Django's existing
code formatting rules.

(An interesting [and defensible] choice, we had a module with a lot of
strings wrapped across lines. black opted to push them onto the same
line, but NOT to merge them.  This is because in Python prior to 3.7, it
would have altered the generated AST - one of the guides black uses)

--
Curtis

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/56392596-0246-b62b-4725-628a2b0801ae%40tinbrain.net.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Proposal to format Django using black

Christian González
Hi
> 2. there are issues with git history in doing "the great commit".

While I don't have much experience neither in Python/Django nor in git,
maybe a view fom "outside" may add something useful to the discussion.

I'm just seeing that discussion is separated between on the one hand
"black is good. It would help Django, newcomers' entry barrier etc." -
and OTOH "black adds a commit to Django that prevents git blame to work
properly."

So, excuse me to blasphemically ask the question: Isn't that a problem
with git? I always asked myself how to properly fix whitespace "bugs"
without destroying the "git blame" history.

What would in my optinion solve that issue, and hence allow black to be
adopted completely in Django without regrets, would be a feature in git
to exclude certain commits, or even chunks, from git blame history. This
could be done by having a .blameignore file like .gitignore, listing
commit hashes that should be ignored in the blame history. So it could
be made clear and configurable per project, and even modified after
committing - because you can't change git history itself - which code
chunks are just "cosmetical" and have not to be treated as real "code".

I knot this has nothing to do with Django itself - but really, NOT
adopting black because of "git blame" creating wrong results would be
the wrong approach IMHO.

If you're not saying that I'm completely insane, or git already has this
feature, or Ii forgot something, I could try and add a git feature
request for that. Even if implemented in 2 years, It could be made sure
later that git blame worked correctly backwards then.

TL;DR: Use black with Django without compromise, fix git.

Christian

--
Dr. Christian González
https://nerdocs.at

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/b689490c-479e-8745-693a-180bdeeed0cd%40nerdocs.at.
For more options, visit https://groups.google.com/d/optout.

pEpkey.asc (16K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Proposal to format Django using black

Florian Apolloner
In reply to this post by Berker Peksağ
Hi,

On Sunday, April 14, 2019 at 1:04:22 AM UTC+2, Berker Peksağ wrote:
I always do it to not bother
contributors with these changes, especially if they are new to the
project.

This is imo not something that scales well. Also it is not something I want to pay our fellows for, I rather have them use their time on something else. It is true that it is probably easier and faster to just fix code instead of back & forth with the contributor, but the main argument for me here is that this is work that shouldn't be needed in the first place.

Cheers,
Florian

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/7b6279fa-1185-45d2-b3d8-bb3f97c00c3f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Proposal to format Django using black

Florian Apolloner
In reply to this post by James Bennett


On Saturday, April 13, 2019 at 10:21:48 PM UTC+2, James Bennett wrote:
But we already have and enforce a style guide, and some parts of it are things Black can't auto-enforce.
 
We do? I mean sure, we do have a styleguide, but enforcing it? It all depends on how well our fellows (manually) enforce it. So even if there are things black cannot auto-enforce, we are currently not enforcing __anything__. And even our great fellows do make mistakes from time to time, something black could prevent (at least in the areas that it could enforce).

Do not get me wrong, I am not trying to bash our fellows here for sometimes making mistakes. But arguing that we are already enforcing a styleguide seems a bit far stretched, even if our fellows are doing an astonishing job there.

Cheers,
Florian

P.S.: I said fellows a few times in this mail, but reviewer/committers would probably be better, even though I think that most of the final fixing is currently on the fellows.

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/64862d83-82d5-4af5-aa35-d473f0ba5639%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Proposal to format Django using black

Florian Apolloner
In reply to this post by Christian González
Hi Christian,

I think you are making a good argument here. On one hand short-comings in our tool chain should not block us from making changes. On the other hand, we do have to live with them -- so having at least some technical solution towards the "git blame" issue is needed. I guess the chromium team also realized this, which led to the development of tools like git-hyper-blame (see https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/git-hyper-blame.html -- to be fair I haven't tried it yet).

Cheers,
Florian

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/1e41a332-f147-4c32-aff9-2c8b6bb3af41%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
1234 ... 6