On Tue, Nov 1, 2011 at 11:31 AM, Fournier, Camille F. <[email protected]> wrote: > 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.
I agree. I committed those - I'll be more diligent in future. (although 1247 looks more like a cleanup than a specific fix) Patrick > -----Original Message----- > From: Patrick Hunt [mailto:[email protected]] > Sent: Tuesday, November 01, 2011 1:15 PM > To: [email protected] > Cc: [email protected]; Benjamin Reed > Subject: Re: cleanup and subjective patches > > On Tue, Nov 1, 2011 at 9:51 AM, Fournier, Camille F. > <[email protected]> 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 >
