Sorry, you're right, let me rephrase: We shouldn't be checking in code that is claiming to fix bugs without tests. I also personally think that we should strongly encourage refactorings to come in with additional tests, as per Ted's suggestion. Leave the code base better than when you left it in a tangible way. There are two patches checked in that Thomas claims are bugfixes, that came in with no tests to verify that the bug they were fixing is actually fixed and had no test in place that was failing due to the bug.
C -----Original Message----- From: Patrick Hunt [mailto:ph...@apache.org] Sent: Tuesday, November 01, 2011 1:15 PM To: dev@zookeeper.apache.org Cc: tho...@koch.ro; Benjamin Reed Subject: Re: cleanup and subjective patches On Tue, Nov 1, 2011 at 9:51 AM, Fournier, Camille F. <camille.fourn...@gs.com> wrote: > Committers, this checking in of code without tests has got to stop. It's > actively detrimental to the code base and undermines any value we might be > getting from refactoring efforts. With very limited exceptions (changing > socket parameters or something else that is virtually impossible to test > without significant investment in additional libraries and scaffolding), we > should not put in ANY more changes without tests. There's a reason we see a > -1 in the Hudson build report for no new tests. Don't take it lightly. There's a difference btw checking in refactorings and fixing warnings and such that are covered by existing test, vs committing feature changes and bug fixes that are not caught by existing tests. Patrick
