Re: [JGIT PATCH] 1/2 : (reworked) Externalizable/Serializable Items

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

Nigel Magnay <nigel.magnay@gmail.com> wrote:
> +public class ObjectId extends AnyObjectId implements Serializable {

We should define our own serialVersionUID:

  private static final long serialVersionUID = 1L;

is good enough to make Java happy.

> @@ -269,4 +273,20 @@ protected ObjectId(final AnyObjectId src) {
>  	public ObjectId toObjectId() {
>  		return this;
>  	}
> +
> +	private void writeObject(ObjectOutputStream os)  throws IOException {
> +	private void readObject(ObjectInputStream ois)  throws IOException {

Minor nit: Only 1 space between ) and throws, please.

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java
> b/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java
> index 5bbf664..899f73f 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java
> @@ -53,7 +57,7 @@
>   * describing how refs should be transferred between this repository and the
>   * remote repository.
>   */
> -public class RemoteConfig {
> +public class RemoteConfig implements Serializable {

Please set a serialVersionUID.

> @@ -382,4 +386,77 @@ public TagOpt getTagOpt() {
>  	public void setTagOpt(final TagOpt option) {
>  		tagopt = option != null ? option : TagOpt.AUTO_FOLLOW;
>  	}
> +	
> +	private void writeObject(ObjectOutputStream os) throws IOException {
> +
> +	    // Name
> +	    os.writeUTF(name);
> +	
> +	    // Key, Value pairs
> +	    for (URIish uri : uris) {
> +            os.writeUTF(KEY_URL);
> +            os.writeUTF(uri.toPrivateString());
> +        }
> +
> +        for (RefSpec refspec : fetch) {

There is some sort of whitespace damage here, the second for loop
is not lined up at the same starting column as the first for loop.
My guess is, you have tabs in here.  We only indent with spaces.

> +            os.writeUTF(KEY_FETCH);
> +            os.writeUTF(refspec.toString());
> +        }
> +
> +        for (RefSpec refspec : push) {
> +        	os.writeUTF(KEY_PUSH);
> +            os.writeUTF(refspec.toString());
> +        }

Should we maybe allow RefSpec to serialize itself with
os.writeObject() rather than using writeUTF() directly?

FWIW, I did find this new implementation to be much easier to read.
Thanks.

-- 
Shawn.