[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