db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "TomohitoNakayama" <tomon...@basil.ocn.ne.jp>
Subject Re: Patch again again for DERBY-167.
Date Wed, 18 May 2005 11:57:19 GMT
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
         tomonaka@basil.ocn.ne.jp
         tomohito@rose.zero.ad.jp

         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
        tomonaka@basil.ocn.ne.jp
        tomohito@rose.zero.ad.jp

        Naka
        http://www5.ocn.ne.jp/~tomohito/TopPage.html

*/
----- Original Message ----- From: "Army" <qozinx@sbcglobal.net>
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


Mime
View raw message