Re: [PATCH JGit 04/19] added utility that generates the contents of the objects/info/packs file as a string from a list of PackFiles

[Robin Rosenberg <robin.rosenberg.lists@dewire.com>]

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PacksFileContentsCreator.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PacksFileContentsCreator.java
> new file mode 100644
> index 0000000..3dd0418
> --- /dev/null
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PacksFileContentsCreator.java
> @@ -0,0 +1,21 @@
> +package org.spearce.jgit.lib;
> +
> +import java.util.List;
> +
> +public class PacksFileContentsCreator {
> +
> +	private List<PackFile> packs;
> +
> +	public PacksFileContentsCreator(List<PackFile> packs) {
We want good javadocs for (at least) all public and protected methods and 
classes. (We enforce this for Eclipse users). If we had similar configuration files
for, perhaps netbeans we might have those too. (not finished thinking about that
proposition...)

> +		this.packs = packs;
> +	}
> +	
> +	public String toString(){

Don't overload toString. Let it be useful for debug like purposes. With this style we cannot
make it useful since the application depends on its exact behaviour. You may define
a toString anyway that does the exact same thing, but please provide a specific method for 
assisting with the formatting of the file. A writeTo(OutputStream) is a useful interface in general.

> +		StringBuilder builder = new StringBuilder();
> +		for (PackFile packFile : packs) {
> +			builder.append("P ").append(packFile.getPackFile().getName()).append('\r');

At least my git formats the file with \n as line terminator, so I think JGit should too.. 
Git ends the file with an extra \n, though I'm not sure it's relevant.

> +		}
> +		return builder.toString();
> +	}

The name is somewhat confusing as the s is hard to spot. The suggestion InfoPacksFileGenerator
perhaps. It's a bit uglier by easier not to mix with generation of pack files.

-- robin