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.

Reply via email to