Re: Patch again again for DERBY-167.

[TomohitoNakayama]

Hello.
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
Firstly I thought make each part as separated method.
But I abort it because separated method will make it difficult to see how 
instance variable of defaultInfo are handled.

I will write a English sentence there.

ColumnDescriptor.java: Is there an extra assert for autoincInc being 
non-zero at line: 156
I will erase unnecessary part...

Also 'static' is not needed for assertAutoinc() method.
As written in reply mail for Army , I prefer static method .....
How should do it ...?
Best regards.
/*
        Tomohito Nakayama
        [EMAIL PROTECTED]
        [EMAIL PROTECTED]
        Naka
        http://www5.ocn.ne.jp/~tomohito/TopPage.html
*/
----- Original Message ----- 
From: Satheesh Bandaram
To: Derby Development
Sent: Wednesday, May 18, 2005 10:00 AM
Subject: Re: Patch again again for DERBY-167.

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" <derby-dev@db.apache.org>
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

No virus found in this incoming message.
Checked by AVG Anti-Virus.
Version: 7.0.308 / Virus Database: 266.11.12 - Release Date: 2005/05/17 


--
No virus found in this outgoing message.
Checked by AVG Anti-Virus.
Version: 7.0.308 / Virus Database: 266.11.12 - Release Date: 2005/05/17