db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Van Couvering <da...@vancouvering.com>
Subject Re: [DERBY-2469] First alpha implementation available - help needed :)
Date Tue, 27 Mar 2007 18:01:03 GMT
Hi, Luigi.  Thanks for submitting this!

Here are my comments:

- Your code is very well written - clear, well-commented, nicely 
structured!

- Sigh.  Does the JNLP persistence service have no concept of a 
directory?  I hate the "length == 0" approach of identifying a 
directory.  But sometimes that's the best we can do.  Perhaps we can 
provide some feedback to the JNLP folks?

- Don't you need to close any open files on shutdown?  This highlights 
my ignorance of this area of the code...

- Next time, please submit your changes as a patch file.  You generate a 
patch file by doing 'svn diff' from the root of your code tree, and 
routing the output to a file.  Make sure you have actually added new 
files and directories in your local tree using 'svn add'.

- You didn't submit any unit tests.  Without this, how do we know this 
feature works?  Have you run it through any tests?

- Did you run derbyall?  Did it pass?

Take a good gander at http://wiki.apache.org/db-derby/DerbyCommitHowTo - 
everything here applies except that you can't commit the change 
yourself, only committers can do that.

- Did you go through the checklist at 
http://wiki.apache.org/db-derby/DerbyContributorChecklist?  This is 
really important, especially signing and faxing in the ICLA

- There are three versions of JNLPStorageFactory.java.  It's really hard 
to tell which one is the most recent.  When submitting updated patches, 
again it's best if you submit a single patch file, and name it with a 
version number or a date.

- Your source files need to have the Apache license header on them.  See 
other files for examples

Gotta run, maybe more later.

Thanks,

David

Luigi Lauro wrote:
> 
> Compiles but doesn't run yet, a lot of things to check/do yet, but a 
> first 90% complete implementation is available.
> 
> You can download the latest versions for the 3 classes here: 
> https://issues.apache.org/jira/browse/DERBY-2469
> 
> I'm coding around a VirtualStorageFactory abstract base class, and a 
> VirtualStorageFile. VirtualStorageFile implements basic logic and 
> delegates to VirtualStorageFactory for the actual storage manipulation.
> 
> VirtualStorageFactory base class is a reworked from scratch 
> 'BaseStorageFactory' which instead of forcing implementors to give both 
> StorageFile and StorageFactory implementation, allows them only to 
> implement a few 'CRUD' create/delete/rename/delete methods, and build 
> upon these to give a working StorageFactory/StorageFile implementation.
> 
> Ideally, if things go as I plan, every storage implementation could be 
> re-written around this new base class if needed, or this could be used 
> as a new base for developing new implementations in a easier way.
> 
> Have a look at the abstract class (VirtualStorageFactory) and the 
> delegating implentation of StorageFile (VirtualStorageFile).
> 
> I also include a starting JNLP implementation of VirtualStorageFactory.
> 
> Javadoc comments comes mostly from interfaces (copy & paste), and VERY 
> FEW TESTS DONE YET, this is very alpha code, if you wanna help go 
> through it and suggest fixes/improvements/whatever to get things rolling 
> for real.
> 
> I've probably done 100000000000000000000000 things wrong, so if someone 
> can help me spot what is right what is not, I can start to fix stuff and 
> provide a beta version soon. Thanks :)
> 
> P.S. Classes names are ugly, I plan to change those soon.
> P.P.S. As soon as I get a beta JNLP implementation out, I will try to 
> provide a in-memory implementation as well. This is very easy to do, 
> given the base class.

Mime
View raw message