Hi there

(I tried to find previous threads concerning the same topic, but could not 
find any. If this is already the n-th discussion about the topic, sorry 
about it.)

I don't quite agree with the cleanup commit policy stated in the docs:

Systematically remove all trailing whitespaces from your code as those add 
> unnecessary bytes, add visual clutter to the patches and can also 
> occasionally cause unnecessary merge conflicts. Some IDE’s can be 
> configured to automatically remove them and most VCS tools can be set to 
> highlight them in diff outputs. Note, however, that patches which only 
> remove whitespace (or only make changes for nominal PEP 
> 8<http://www.python.org/dev/peps/pep-0008>conformance) are likely to be 
> rejected, since they only introduce noise 
> rather than code improvement. Tidy up when you’re next changing code in the 
> area.


I did such a PR today (without knowing that policy): 
https://github.com/django/django/pull/1345

While I agree that small PRs which fix issues like whitespace should not 
necessarily clutter up the commit history, I disagree for larger cleanup 
commits. In some places the code has serious formatting issues (e.g. lines 
that are indented 3 instead of 4 characters and that only work thanks to 
the lax Python indentation parsing). Also, I disagree that 1 commit which 
cleans up a file would "clutter" its commit history.

While I support fixing coding standard issues on-the-go, I think that it 
clutters up the commit history in a worse way than a cleanup commit would, 
because the commits are not strictly focused on the feature or bug they 
concern.

In addition to the PEP8 changes there were also a few pyflakes changes that 
go beyond simple whitespace formatting. For example in the core module 
there were a few places where "variable == None" was used, even though 
"variable is none" should be used preferredly. Another issue would be 
"type(c) == Typeclass" instead of "isinstance(c, Typeclass)".

Anyways, if you don't want to accept such commits that's OK, but I think 
adherence to coding standards is pretty bad in many Django modules and it 
should be fixed. And for sure I won't be the last person to send you such a 
pull request.

Danilo

-- 
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 [email protected].
To post to this group, send email to [email protected].
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