Re: [JGIT PATCH v2 05/24] Added WorkTree class which can be constructed over Repository.

["Shawn O. Pearce" <spearce@spearce.org>]

Florian Koeberle <florianskarten@web.de> wrote:
> +	public static WorkTree createWorkTree(File workTreeDirectory)

I'm only going to make this remark once for your series: In general I
prefer to mark any method argument, instance field or local variable
that I do not plan on changing within the method or class final.
This has saved me more than once from stupidly changing a parameter
when I meant to change something else.

Its not a hard and fast rule in either jgit or egit; you don't need
to go back and fix every single site.  Just something I have found
useful over the years and now do almost everywhere.

> +			throws IOException {
> +		final File gitDirectory = new File(workTreeDirectory,
> +				Constants.REPOSITORY_DIRECTORY_NAME);
> +		if (gitDirectory.exists()) {
> +			throw new IllegalArgumentException(
> +					"Repository exists in given project directory.");

When we throw an exception like this we should include the path
we are complaining about already existing as part of the message
(see Repository.create() for example).  This way if the message
makes it as far as the end-user (which it probably should, and
then there's translation issues are currently ignoring in jgit)
the user can at least see what path we were trying to operate on.
It may help them to realize they are in the wrong directory, or
made a typo on an input, etc.


Another thing about this method is we cannot supply the WindowCache
that the Repository should use for object data access, which means
this Repository will have its own WindowCache.  Under Eclipse (or
any other larger GUI/IDE) we really want only a single WindowCache
instance for the entire JVM as the heap must be shared across all
Repository instances.

This is actually making me reconsider how the WindowCache is handled.
Maybe we should have a single WindowCache instance down inside
of jgit, but allow the higher level UI (like Eclipse) to tune it
on the fly.  Then we don't have to worry about passing it down to
utility routines like this anytime we open a Repository.

Hmmmm.

-- 
Shawn.