Relevant discussion https://groups.google.com/forum/#!msg/django-developers/tcNVvbAv4-M/bs0zPNLqv48J
Cheers, Tom Dne Mon, 8 Jul 2013 09:59:49 -0700 (PDT) Danilo Bargen <[email protected]> napsal(a): > 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 >
signature.asc
Description: PGP signature
