Substitution (aka aliases)

[Mandy Chung]

On 4/12/2012 11:47 AM, Alan Bateman wrote:
> I went through all the changes, good work! It's nice to get rid of the 
> directories for the views along with the copy of the module info classes.
>

Thanks for the review, Alan.

> As you pointed out, most of the changes are in SimpleLibrary so I 
> spent the most time there.
>
> On the lock file, it only need only be opened for writing to get an 
> exclusive lock.
>
> What would you think of Dictionary instead of ModuleDirectory for the 
> inner case name (Catalog comes to mind too but that's already used)? 
> Could it extend MetaData too?
>

As for class name, I also thought about "Dictionary" while I think it's 
more like a directory of looking up names.  I can change it if you think 
Dictionary is a better fit.

I also like to make ModuleDirectory and also RepoList inner classes to 
extend MetaData but it requires some cleanup that I leave it for later 
to do.  The current MetaData class writes to the file directly but what 
we want here to write to a temporary file first and then atomically 
replace the file so that readers of ModuleDictionary doesn't need to 
hold the exclusive lock.  I keep this as one of the cleanup tasks to do.

> Minor comment but the private modifier on every method is a bit of a 
> distraction in ModuleDirectory.
>

Agree - I'll clean them up.

> ModuleDirectory.refresh could use DirectoryStream to iterate over the 
> directory and would avoid the canRead check (which isn't reliable on 
> all platforms anyway). The newDirectoryStream method will fail if the 
> directory can't be opened which is what is needed.
>

I'll take a look at that.

> A couple of minor nits, inconsistent indent in Catalog.java L143, 
> RemoteRepository.java L337, Commands.java L182. One other nit is the 
> copyright date on hello-alias.sh.
>

Will fix them.   Thanks for catching them.

> Otherwise I think this is good. As you noted, the main open question 
> here is whether aliases should have version numbers.
>

Thanks.
Mandy