Burn the backslashes! On a more serious note, I think these need to be taken on a case by case basis. Certainly unhelpful it test code though. On 21 Sep 2013 18:50, "Aymeric Augustin" <aymeric.augus...@polytechnique.org> wrote:
> Hello, > > I think we went overboard in this commit: > https://github.com/django/django/commit/165f44aa > > Specifically, I consider it's a regression in three areas. > > > 1) Readability > > After the patch, indentation no longer matches the logical structure of > the program. For example: > > - with self.settings(USE_L10N=True, USE_THOUSAND_SEPARATOR=False): > - with translation.override('en'): > - self.humanize_tester(test_list, result_list, 'intcomma') > + with self.settings(USE_L10N=True, USE_THOUSAND_SEPARATOR=False), \ > + translation.override('en'): > + self.humanize_tester(test_list, result_list, 'intcomma') > > The new version is longer than the old one (by two characters) and > visually harder to parse. > > > 2) Readability (bis) > > After the patch, distinct operations end up on the same line, obfuscating > the intent of the code. For example : > > - with six.assertRaisesRegex(self, Exception, "Oops"): > - with transaction.atomic(): > - Reporter.objects.create(first_name="Haddock") > - raise Exception("Oops, that's his last name") > + with six.assertRaisesRegex(self, Exception, "Oops"), > transaction.atomic(): > + Reporter.objects.create(first_name="Haddock") > + raise Exception("Oops, that's his last name") > > In the old version, the first line is a test assertion and the second line > is the code exercised by the test. I find it confusing to put both on the > same line. > > > 3) Consistency > > Django always uses implicit line continuations instead of backslashes. > This commits introduces several explicit line continuations with > backslashes that can't be turned into implicit ones. > > > To be fair, sometimes the new version is more readable : > > - with translation.override('ja'): > - with self.settings(USE_L10N=True): > - self.humanize_tester([100], ['100'], 'intcomma') > + with translation.override('ja'), self.settings(USE_L10N=True): > + self.humanize_tester([100], ['100'], 'intcomma') > > But whenever the with statement spills over two lines, which happens in a > majority of cases, I find it worse than two with statements. It's > especially bad in the transactions tests — I worked in this area today. > > -- > Aymeric. > > > > -- > You received this message because you are subscribed to the Google Groups > "Django developers" 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 http://groups.google.com/group/django-developers. > For more options, visit https://groups.google.com/groups/opt_out. > -- You received this message because you are subscribed to the Google Groups "Django developers" 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 http://groups.google.com/group/django-developers. For more options, visit https://groups.google.com/groups/opt_out.
