Hello Alex, > Could we do the work to make black (or another suitable formatter) apply only > to a range of lines? Prettier has this feature[0], which is used by > precise-commits[1] in the way we'd want to use it for Django. It takes a > range of lines, expands the range to statement boundaries, and formats only > those lines. This feels like a solution nearly everyone could agree on and > would be an asset to the Django and broader Python community.
Yes, that would alleviate about half of the concerns — the other half being opposition to some style choices made by Black. The answer to "could we do the work" is pretty much in your hands :-) If you want this to happen, put together a PoC and write DEP 9. If I see good momentum on a credible alternative, I'm willing to put DEP 8 on standby for a few weeks. > From my perspective, the DEP as it stands undervalues blame and Django's > commit history. In PyCharm, the date, message and entire diff of the commit > responsible for every line is immediately available, in the margin of the > editor or with a keyboard shortcut. When it's a trivial to get to, this > context is incredibly useful. This is a good point. It's valid insofar as the last commit contains useful information by itself, which requires some luck in my experience with Django. I triaged a few thousand tickets between 2011 and 2014. Back then many git blame expeditions required jumping through several commits. If I needed git blame to understand the code, often it meant that two different people touched the same piece of code and left an inconsistency, so the last commit by itself wasn't going to give me the answer. I don't think this changed since then. That said, I don't have the kind of setup you describe, and if I had, I would certainly lean on git blame more heavily. > To address the counter-arguments: the "view blame prior to this change" > button in GitHub shows you the blame for the entire file before the specified > commit, as opposed to the blame for the file in its current state, > disregarding the specified commit. Not bad, but not quite what you want. > git-hyper-blame has been raised several times, but it's non-standard, not > straightforward to install, and not supported by tooling. A git patch was > mentioned that incorporates hyper-blame's functionality into blame[2], which > looks like it'll be great, but won't help until it's merged, released, and > integrated into tooling. Yes, let's be honest: DEP 8 doesn't have a much better argument than "oh well we'll survive with the tools we have". > I'm firmly -1 to globally applying black to the codebase until it can be done > without breaking blame. By framing the problem in that way, you imply that git blame currently works and that it will stop working if Black is applied to the codebase. Both are false :-) Rather, we'd be trading one problem for another : - The current problem is random small reformattings, which I referred to as "a steady stream of pollution" in DEP 8 and which I experienced very regularly when I was an active contributor ; this is not a theoretical issue ; - The new problem if DEP 8 goes live is the Big Commit ; I find it easier to determine that a commit isn't significant if the message says "Reformat Django with Black" so the new problem could be easier to deal with than the current problem. Best regards, -- Aymeric. > On 29 Apr 2019, at 05:11, Alexander Hill <a...@hill.net.au> wrote: > > Hi all, > > Could we do the work to make black (or another suitable formatter) apply only > to a range of lines? Prettier has this feature[0], which is used by > precise-commits[1] in the way we'd want to use it for Django. It takes a > range of lines, expands the range to statement boundaries, and formats only > those lines. This feels like a solution nearly everyone could agree on and > would be an asset to the Django and broader Python community. > > From my perspective, the DEP as it stands undervalues blame and Django's > commit history. In PyCharm, the date, message and entire diff of the commit > responsible for every line is immediately available, in the margin of the > editor or with a keyboard shortcut. When it's a trivial to get to, this > context is incredibly useful. Plugins for vim and other tools provide similar > functionality. GitHub is pretty good in this regard too, but it really shines > when integrated into your editor. Like auto-formatting, I think you don't > know how good this is until it's part of your workflow. > > To address the counter-arguments: the "view blame prior to this change" > button in GitHub shows you the blame for the entire file before the specified > commit, as opposed to the blame for the file in its current state, > disregarding the specified commit. Not bad, but not quite what you want. > git-hyper-blame has been raised several times, but it's non-standard, not > straightforward to install, and not supported by tooling. A git patch was > mentioned that incorporates hyper-blame's functionality into blame[2], which > looks like it'll be great, but won't help until it's merged, released, and > integrated into tooling. > > Django is a mature project, which means that many, many lines have remained > unchanged for years, and will continue to be attributed to the black > mega-commit for many years to come. > > I'm firmly -1 to globally applying black to the codebase until it can be done > without breaking blame. > > Alex > > [0] https://prettier.io/docs/en/options.html#range > <https://prettier.io/docs/en/options.html#range> > [1] https://github.com/nrwl/precise-commits > <https://github.com/nrwl/precise-commits> > [2] > https://public-inbox.org/git/CAJDYR9SL9JCJjdARejV=NCf9GYn72=bfszxx84idc416szm...@mail.gmail.com/T/ > > <https://public-inbox.org/git/CAJDYR9SL9JCJjdARejV=NCf9GYn72=bfszxx84idc416szm...@mail.gmail.com/T/> > On Sun, Apr 28, 2019 at 10:51 PM Aymeric Augustin > <aymeric.augus...@polytechnique.org > <mailto:aymeric.augus...@polytechnique.org>> wrote: > Hello, > > Here's my attempt at summarizing the conversation in a DEP: > https://github.com/django/deps/pull/55 > <https://github.com/django/deps/pull/55>. > > It's easier to read as a rich diff: > https://github.com/django/deps/pull/55/files?short_path=95a1a7b#diff-95a1a7b430e2608b84f5c834fd6c258c > > <https://github.com/django/deps/pull/55/files?short_path=95a1a7b#diff-95a1a7b430e2608b84f5c834fd6c258c> > > Please let me know if I missed or represented unfairly some ideas! > > Best regards, > > -- > Aymeric. > > PS: while I'm eager to listen to feedback and iterate on this draft, I would > also prefer if this DEP didn't turn into a novel, so let's avoid going into > every detail and trust the implementation team to do a good job — thank you > :-) > > >> On 13 Apr 2019, at 13:52, Herman S <herman.schis...@gmail.com >> <mailto:herman.schis...@gmail.com>> 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 >> <https://github.com/hermansc/django/pull/1> >> * Line length kept at 119: https://github.com/hermansc/django/pull/3 >> <https://github.com/hermansc/django/pull/3> >> * Line length kept at 119, no string normalization: >> https://github.com/hermansc/django/pull/2 >> <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 <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-developers+unsubscr...@googlegroups.com >> <mailto:django-developers%2bunsubscr...@googlegroups.com>. >> To post to this group, send email to django-developers@googlegroups.com >> <mailto:django-developers@googlegroups.com>. >> Visit this group at https://groups.google.com/group/django-developers >> <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 >> >> <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 >> <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 > <mailto:django-developers+unsubscr...@googlegroups.com>. > To post to this group, send email to django-developers@googlegroups.com > <mailto:django-developers@googlegroups.com>. > Visit this group at https://groups.google.com/group/django-developers > <https://groups.google.com/group/django-developers>. > To view this discussion on the web visit > https://groups.google.com/d/msgid/django-developers/ECC90690-D27E-40D3-B69E-765D988A1690%40polytechnique.org > > <https://groups.google.com/d/msgid/django-developers/ECC90690-D27E-40D3-B69E-765D988A1690%40polytechnique.org?utm_medium=email&utm_source=footer>. > For more options, visit https://groups.google.com/d/optout > <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 > <mailto:django-developers+unsubscr...@googlegroups.com>. > To post to this group, send email to django-developers@googlegroups.com > <mailto:django-developers@googlegroups.com>. > Visit this group at https://groups.google.com/group/django-developers > <https://groups.google.com/group/django-developers>. > To view this discussion on the web visit > https://groups.google.com/d/msgid/django-developers/CA%2BKBOKyL3Y3Hd2pFOzq_x%2BwPLKp%3D4%2BGjS9cXJddZQZv8cG-ZuA%40mail.gmail.com > > <https://groups.google.com/d/msgid/django-developers/CA%2BKBOKyL3Y3Hd2pFOzq_x%2BwPLKp%3D4%2BGjS9cXJddZQZv8cG-ZuA%40mail.gmail.com?utm_medium=email&utm_source=footer>. > For more options, visit https://groups.google.com/d/optout > <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/0D72AAEC-4A7B-472D-B2EC-5F6506722079%40polytechnique.org. For more options, visit https://groups.google.com/d/optout.
