[Zope3-dev] Re: Bogus change to zope.app.rdb
Sidnei da Silva
sidnei@redesul.com.br
Fri, 4 Jul 2003 09:57:16 -0300
On Fri, Jul 04, 2003 at 03:35:52PM +0300, Steve Alexander wrote:
| Here's a recent checkin to the zope.app.rdb package.
|
| http://www.codeworks.lt/zope3-checkins/%3C200307032246.h63Mklk01481@cvs.baymountain.com%3E
|
| The checkin is bogus for a number of reasons.
|
| * The change was made without an accompanying unit test to demonstrate
| the bug that was being fixed. This is very bad, as a maintainer of the
| code now has no idea why the code is more complex than necessary.
|
| * The use of try: except: TypeError is way too broad. There could be
| TypeErrors inside the operation of a converter. This would give very odd
| results, and would hide errors.
|
| * The question in comments in the code is:
|
| +## XXX Geez! This was badly broken for me, and srichter was not
| +## around to explain what this code is supposed to do when
| +## theres only one row/one column on the result.
|
| Why should a one-item row or column be a different type than a mult-item
| row or column? In either case they should be sequences. There should be
| no special case for one-item rows or one-item columns.
|
| This checkin is bogus. Please revert the change until at the very least
| you can provide a unit test for it.
The problem is: there is a test for the behavior, but It is done based
on dummy data, and not a real rdb connection, so I cant really tell if
the problem is a fault of the mysqldb connector or if the test is
broken.
Anyway Ill revert the checkin. If anyone is able to help me figure
out, I would be glad.
[]'s
--
Sidnei da Silva (dreamcatcher) <sidnei@x3ng.com.br>
X3ng Web Technology <http://www.x3ng.com.br>
GNU/Linux user 257852
Debian GNU/Linux 3.0 (Sid) 2.4.20-powerpc ppc
To the systems programmer, users and applications serve only to provide a
test load.
-----------------------------------------------------------------------
Verified for virus by mail.redesul.com.br
Scanner: clamscan / ClamAV - Version 0.54 - Updated 01/07/2003