From: Robin Rosenberg <robin.rosenberg@dewire.com>
Date: Thu, 21 May 2009 15:22:33 GMT
Subject: Re: [EGIT PATCH 1/6] Make sure we get the right storage for loose/pack/loose and packed refs
Message-ID: <200905211722.33995.robin.rosenberg@dewire.com>
In-Reply-To: <20090520214359.GQ30527@spearce.org>

onsdag 20 maj 2009 23:43:59 skrev "Shawn O. Pearce" <spearce@spearce.org>:
> Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > Also adds a few some more test for refs, though not complete.
> 
> Hmm, tests fail, wrong expected values?

Gah, not sure how that passed. It's not random failures...

> Actual was Result.REJECTED_CURRENT_BRANCH.
> 
> > @@ -349,12 +355,20 @@ private synchronized Ref readRefBasic(final String origName,
> > +
> > +		if (!origName.equals(name)) {
> > +			ref = new Ref(Ref.Storage.LOOSE, origName, name, id);
> > +			looseRefs.put(origName, ref);
> > +		}
> 
> Seems fine, but I'm starting to hate our current way of handling
> symrefs.  Not for this series.  But its starting to worry me.

I agree. For decorating the history it's convenient, but it's inconsistent. For some reason
it was harder to see without the unit tests. All kinds of things slip by when testing is
done on the surface only.

I'll update, and and even more tests. 

Should we use multiple Ref objects for symrefs? I.e. a Ref referring to another in a chain,
with all symrefs in between visible?

-- robin