Thanks for applying review comments. Overall, the patch looks good.
Some minor comments:
- ColumnDefinitionNode: I would like to see following comments be
more descriptive. Initially I thought these refer to another method,
but that is not the case.
- + //validateDefaultOfAutoInc
+ //validateDefaultOfDefault
- ColumnDescriptor.java: Is there an extra assert for autoincInc
being non-zero at line: 156? (Second constructor). Also 'static' is not
needed for assertAutoinc() method.
Please apply Army's and my comments. I also invite others to review the
patch, since I will be looking to commit this one soon, after getting
an updated patch. :-)
Satheesh
TomohitoNakayama wrote:
Hello.
I send new patch for DERBY-167 , which is attached to this mail.
Please review it.
Best regards.
/*
Tomohito Nakayama
[EMAIL PROTECTED]
[EMAIL PROTECTED]
Naka
http://www5.ocn.ne.jp/~tomohito/TopPage.html
*/
----- Original Message ----- From: "Army" <[EMAIL PROTECTED]>
To: "Derby Development" <[email protected]>
Sent: Saturday, May 14, 2005 9:13 AM
Subject: Re: Patch again for DERBY-167.
TomohitoNakayama wrote:
Hello.
I send new patch for DERBY-167.
Please review it again.
I tried to apply this patch to my local codeline (which I just updated)
so
that I could review it more closely, but the patch fails to apply in
several places.
I know it's annoying, but could you perhaps "svn update" your codeline
and
then re-create the patch based on the latest files?
If I can get the patch to apply, it makes it easier to review the code.
Also, I can then run the new/updated tests and verify that everything
works as intended...
Thanks,
Army
--
No virus found in this incoming message.
Checked by AVG Anti-Virus.
Version: 7.0.308 / Virus Database: 266.11.10 - Release Date: 2005/05/13
No virus found in this outgoing message.
Checked by AVG Anti-Virus.
Version: 7.0.308 / Virus Database: 266.11.10 - Release Date: 2005/05/13
|
- Re: Patch again again for DERBY-167. Satheesh Bandaram
-
