Re: [RFC][StGit PATCH] Add support for merge-friendly branches

From: Karl Hasselström <kha@treskal.com>
Date: May 29, 2009, 09:12 UTC
Message-ID: <20090529091208.GA12039@diana.vm.bytemark.co.uk>
In-Reply-To: <b0943d9e0905290140i57aeaa33r8de34302b63ffa5e@mail.gmail.com>

On 2009-05-29 09:40:59 +0100, Catalin Marinas wrote:

Show 15 quoted lines

> 2009/5/29 Karl Hasselström <kha@treskal.com>:
>
> > On 2009-05-28 15:51:20 +0100, Catalin Marinas wrote:
> >
> > > 2009/5/28 Catalin Marinas <catalin.marinas@gmail.com>:
> > >
> > > What about this change to the original patch (it's faster to
> > > just return the sha1 refs than building the Commit objects):
> >
> > Creating Commit objects is really cheap---just look at the
> > constructor. I made them that way on purpose, so that we'd never
> > have to think twice about using Commit objects instead of passing
> > sha1s around.
>
> I was worried it may invoke git to get the CommitData.

Only if you try to access it---it's evaluated lazily.

Show 7 quoted lines

> > Also, you said "set", and I agree---the return value of
> > get_mege_bases() should be a set. That's what it _is_,
> > conceptually, and it makes little sense to obscure that fact.
>
> If we return a set of commits, I suspect the Repository object
> guarantees that having the same sha1 value always returns the same
> Commit object

Yes, exactly. As long as you get all your Commit objects from the Repository object like you're supposed to, there'll be at most one Commit object for each sha1. In fact, the Commit objects don't have an __eq__ method on purpose---straight object comparison already does exactly what we want!

> and the code below is valid:

Yes, this is precisely what I meant.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle