Created attachment 15038 [details]
The description of improvements.

The API review will be done on Monday, May 31.

I'm lacking some crucial information:
1) It would be good to mention where the sources can be found - it's
branch "cache_32089_1" of vcscore.

2) It would be nice if the javadoc would be provided for the reviewed
sources. Building of the javadoc for the sources from the branch fails
because the branch is not reflect the current build infrastructure in
the trunk. I'll try to checkout and build javadoc in the release36
sources environment - that should likely work.

3) What will be stability classification of your API according to
http://openide.netbeans.org/tutorial/api-design.html ?

4) The provided document is useful but I would appreciate an
arch.xml-like document as well. Either current arch-vcscore.xml could
be extended but only in case that your API would have Private
stability. More preferably a separate document should be created.

Talked to Martin E. today - he confirmed that the branch is not
buildable. The javadoc can only be built for the
org.netbeans.modules.vcscore.cache package manually.

API questions/notes:

M01 - class Cache:
 - looks like there is API and SPI mixed by having CacheReader and
CacheWriter in the Cache constructor. Would it be possible to have a
separate class e.g. spi.CacheImplementation that would back the Cache
(Cache's protected methods etc. could be moved there).
 In consequence the CacheHanlder.registerCache()/unregistedType()
(should be unregister??) would move to spi's support.

M02 - Cache.getDir(String path, int level);
 - are the clients of the API supposed to choose the level? Shouldn't
that rather be a part of the impl/spi?

M03 - "level" parameter:
 I think that we should have some broader discussion regarding
providing the "level" parameter to various API methods.

M04 - CacheDir:
 Not sure whether CacheDir should be splitted as well - like Cache
class (M01). There are protected firePropertyChange() methods so you
could consider moving them to a spi.CacheDirImplementation and make
the CacheDir final.

M05 - CacheDir.getDirs(), getDirNames(), getFileNames():
 Consider whether you don't want to return collections instead of
arrays - collections can be made unmodifiable in opposition to arrays.
You should at least notice that the arrays must never be modified by
the clients.
 Would be worth to clearly note in javadoc whether the methods operate
operate on files/dirs recursively or not.

M06 - CacheDir.getAttributes() - improve javadoc:
 I was not able to assume the operation of the two methods from their
javadoc (e.g. what's the "name" parameter?; does the second method
return null when not in memory level?):

public CachedAttributes getAttributes(java.lang.String name)
    Get the cached attributes of a file.

public CachedAttributes[] getAttributes()
    Get the array of attributes, that are currently in the memory level.

    Returns:
            The array of attributes, or null if the list of children
was not loaded yet.

M07 - DefaultDiskReaderWriter:
  Who is the client for this class? Shouldn't this be move to
impl/spi.support?

M08 - Consider creation of  "spi.support":
  Consider mvoing supprot classes from spi package to spi.support to
better separate what SPI clients are supposed to implement and what
are the support classes that they can reuse/extend. By a brief look I
guess the CacheUpdater and Default* classes are likely support ones.

Support for 3 (the stability), M07 and the request for arch*.xml
document. I have not read the javadoc, as for inception that is
probably not necessary, but I want to know answers to following questions:

Y01 Will the cache behave differently for different version control
systems? E.g. I know about a suggestion that CVS should use no cache
by default. Possible? Desirable? Will it be implemented?

Y02 What files are going to be written a read? Where the cache is
going to be stored (one file, tons of .nbintdb, etc.)? 

Y03 Who is going to use your API? Or in different way: Can part of
this API be accessed by FileObject.getAttribute or any other part of
the FS api?

Y04 I would strongly suggest to invest a time _prior_ to any fixes to
get the FS testing infrastructure running on vcscore and all for of
VCS profiles. I have seen that it is pretty easy to reuse existing
tests for a filesystem as done by openide/masterfs and having any test
for VCS is 1000% improvement over current state.

Y05 I am not sure what a "central error reporting mechanism" from a
1-pager is supposed to mean? ErrorManager?

ad M05 - another advantage of Collection is that they can return
lazily initiated objects. With arrays you have to populate it
completely before the method returns. The approach with arrays don't
scale well.

According to Martin's answers (Martin correct me if I'm wrong or
append additional info):
Ad Y01:
 Cache should behave in the same way for different VCSs.
 Without cache the "needs patch" flag could not displayed - would be
regression.

Ad Y02: 
 For CVS the cache should be a file under CVS subdir of each versioned
folder.
 For other VCSs there should be a cache files structure that mimics
the versioned directories.

Ad Y05:
 In certain situations it's desirable to collapse many IOExceptions
into just one. IOException should handle this. It's a central
functionality (not VCS-profile specific).

Opinion document draft is available at
http://openide.netbeans.org/tutorial/reviews/opinions_43637.html

Re. do we need VCS filesystem at all see my comment at
http://www.netbeans.org/servlets/ReadMsg?msgId=769832&listName=nbdev

FYI 

I'm working on cache implementation simplification. It uses Map like
interface: 
  + void set(FileKey, FileStatus)
  + FileStatus getCached(FileKey)
  + void add/removeListener(l)

and utility methods:
  + FileStatus get(FileKey)  // may connect to repository
  + FileStatus getFresh(FileKey) // connects and caches result

and Utility class IgnoreList   
  + bool isIgnored(FileKey)

There is also AbstractFileSystem integration method that allows to
trace FileObject's lifetime (and derived FileKey lifetime).
  + FileReference createFileReference()

FileKey is currently FileObjct itself but it seems to lack uniquess
and I'll replace it by FileUtil.toFile(fo).getAbsolutePath().
Prototype is being developed in vcscore.turbo package.

If it prove successfull it'd affect this issue a lot.

Context <http://vcscore.netbeans.org/nonav/doc/cache/context.html>
describes design feedback based on implementing the prototype.

The new initial review will be held on Oct 20.

The opinion document
http://openide.netbeans.org/tutorial/reviews/opinions_43637.html
was updated by the second round of inception review.

The final review is scheduled to  2004/12/01 which is getting very
close and I still see a lot of TC?s being unresolved. What is the status?

Summarized in
<http://vcscore.netbeans.org/doc/cache/arch-diff-20041124.html>.

TCR: I have generated the javadoc and it contains scary information.
Tons of packages without any comment, apichanges stolen from diff
module and arch.xml that does not even mention turbo and its usage
(for example lookup).

TCR: There is so much API classes and no usecase. Describe why each of
the classes has to be there by showing usecases that is using it. The
package is not self contained, it references other vcscore packages

TCR: Javadoc is not published. 

TCR: FileAttributeProvider.MemoryCache should not be there. Replace
with factory method.

TCR: Tests cannot be executed:
/usr/local/home/jarda/src/nb_all/vcscore/test/build.xml:54: The
following error occurred while executing this line:
/usr/local/home/jarda/src/nb_all/vcscore/test/build-qa-functional.xml:90:
The following error occurred while executing this line:
/usr/local/home/jarda/src/nb_all/xtest/lib/module_harness.xml:361:
Cannot find plugin resource. Reason: Plugin 'deprecated' not found.
Please install it.

- Have you generated right Javadoc? Explicitly select turbo and its
subpackages.

- Probably the same problem. vcscore != VCS Cache. You cannot follow
public packages mentioned in manifest. These are in error (too
verbose) that cannot be easily fixed.

- Correct, no plan to publish any Javadoc.

- As designed. Under full turbo.local package control.

- You need to setup your environment. It works in my IDE. Yes, I plan
to ant-ify them later on.


Thanks for the feedback!

antify and make sure it is included in daily tests, run by Petr Zajac.
Mila please mention in your opinion that the tests should be run
daily, please.

I have update opinion document
http://openide.netbeans.org/tutorial/reviews/opinions_43637.html and
added a dependency on the TCR issue #52157.

The review itself is over.