On Wed, Jul 31, 2013 at 3:33 AM, Joachim Durchholz <[email protected]> wrote: > Am 30.07.2013 15:55, schrieb F. B.: > >> The test of code quality already performs a check of no trailing-space and >> just 1 new line at the end of the file in sympy's code. >> >> What about adding more tests? >> >> The lack of conformance may cause merge problems in old PR. > > > I'm not aware of any problems with that in the past. > > >> What about adding the space requirement in comma separated lists? > > > This could become a problem if somebody modified that across all files. > So we don't do that... > > >> What about a constraint on the number of newlines inside a file? > > > These don't create merge problems. > > > There would be a massive merge problem in the transition phase, people would > need to revisit their existing code before their pull requests could be > merged. > So to make that code work, there would need to be a bin/pep8 tool that > automatically reformatted the code to whatever conformance the newly added > pep8 tests require. That tool would also have to be integrated into the pull > request workflow somewhere; pushing for more PEP 8 conformance requires > identifying a suitable spot for that. > > Another problem with PEP 8 conformance is that it raises the entry barrier > for new contributors, since they need to polish their code before they can > even test it. So the tool needs to be integrated in a way that is as > unobtrusive as possible (say, the failing test could tell them the exact > incantation for bin/pep8 to make the test pass; automatically running > bin/pep8 in preparation to tests would be less obtrusive but could damage > code so bin/pep8 needs to be extra careful, maybe by saving backups or > something).
I agree with this. We encourage new contributors to write high quality code, but if someone is already having difficulty with git, this just makes things worse. Trailing whitespace and extra or zero newlines at the end of the file are bad because git complains about them, and they lead to very confusing (not to mention noisy) diffs when they are changed. Aaron Meurer > > These points need to be addressed to avoid a -1 from me. > > > -- > You received this message because you are subscribed to the Google Groups > "sympy" 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/sympy. > For more options, visit https://groups.google.com/groups/opt_out. > > -- You received this message because you are subscribed to the Google Groups "sympy" 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/sympy. For more options, visit https://groups.google.com/groups/opt_out.
