FWIW, I like that we have high standards for concise and readable comments with proper grammar, especially because I myself often need a little extra push in that department. And I'm not easily convinced by the arguments that sound a whole lot like "I've written the code and don't have time to clean this up." In the thick of the moment it's easy to assume a solution will be obvious to others reading the code, when in reality taking a few extra minutes to clearly write down *why *something is the way it is (not to mention making the code itself readable in the first place) will save potentially hours of someone else's time later on. This is precisely what code reviews are for; to make sure we're handing down well-written, described, and documented code for those who come after us (including perhaps ourselves, a year or two down the road). The benefit to us as contributors is that it makes us stronger developers, and I think that might get forgotten sometimes.
I've continued to follow along in this thread and remain fairly ambivalent about Black. I see how it will simplify many things; on the other hand, I like my multi-line lists formatted my way. (Not poking fun, I really do.) That said, if Black will truly remove some significant hurdles, let's do it, but we should remain clear-headed about the fact that it's not going to remove from the equation what some may feel is nitpicking and others would argue is a necessary part of maintaining our high standards. Herman, if you haven't been scared away from this just yet, I might suggest trying to summarize the discussion and your recommendation/proposal given the reactions so far (could be a DEP <https://github.com/django/deps> but doesn't have to be, IMO). I do feel there's a pragmatic solution in here somewhere. Best to all, Tobias *Tobias McNulty*Chief Executive Officer tob...@caktusgroup.com www.caktusgroup.com On Fri, Apr 19, 2019 at 8:52 AM Tim Graham <timogra...@gmail.com> 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 >> <https://docs.djangoproject.com/en/2.2/internals/contributing/writing-code/coding-style/> >> 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. >> >> On Sat, Apr 13, 2019 at 6:35 PM Herman S <herman....@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 >>> * 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 >> >> https://blog.jani.tiainen.cc/ >> >> 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 >> <https://groups.google.com/d/msgid/django-developers/CAHn91ofkXhofPR_3bjSg2Swei2SyQ_axo6Ymy2dSuKg8G878GA%40mail.gmail.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 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 > <https://groups.google.com/d/msgid/django-developers/06cf99d0-0521-4441-9252-86ef7f779039%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 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/CAMGFDKSg1yN8NQD831N9_NNmmXxBab1gG1z3aijmsBiApu6H1g%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
