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.
