http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10508 --- Comment #25 from Galen Charlton <gmcharlt@gmail.com> --- (In reply to Srdjan Jankovic from comment #24)
(In reply to Galen Charlton from comment #23)
Similar to the concerns I expressed in bug 10528, I have reservations about a patch that both changes a bunch of core routines and adds tests. Please split it up.
Why? I think it is a good thing to add tests that support the change. Makes it kind of complete
My statement, taken out of the context of this bug, was indeed ill-worded. Of course I want tests to accompany patches that change core routines (although I would prefer that the tests, including regression tests, be in a separate patch from the one that changes the routines). But consider it in the context of this bug, which advertises itself as "UT: C4::Branch.pm needs unit tests", not "there are inappropriate uses of $sth->finish() in C4::Branches". In other words, this is part of the ongoing process to improve test coverage, and it's mixing up *that* with changes to core routines that I object to. Sure, in the process of writing tests for a module, it's quite likely that one will run into cases where a routine ought to be improved -- but those should be handled separately, either via separate bugs or at least separate patches. -- You are receiving this mail because: You are watching all bug changes.