dulwich team mailing list archive

[Augie Fackler <durin42@xxxxxxxxx>]

On Thu, May 20, 2010 at 12:13 PM, Jelmer Vernooij <jelmer@xxxxxxxxx> wrote:
> On Wed, 2010-05-19 at 20:12 -0500, Augie Fackler wrote:
>> On May 19, 2010, at 7:59 PM, Jelmer Vernooij wrote:
>> > On Wed, 2010-05-19 at 19:02 -0500, Augie Fackler wrote:
>> >> On May 19, 2010, at 6:46 PM, Jelmer Vernooij wrote:
>> Please do, it causes errors on push if nothing needed to be pushed
>> (6c07a270e99122e722f5a1e56289596156d3a2d4, but you'll need to remove
>> the test change since that assumes my other patches, I can split the
>> change if you'd like).
> If you can do that, that'd be great. :-)

I'll ping you when I get that done, probably this evening US time.

>> > but the GitClient
>> > refactoring needs more discussion and I don't want to postpone the
>> > release further because of that.
>> (disclaimer: this feels like it comes across as pushy - that's not my
>> intent - I merely want to try and have a shot at fixing these for the
>> release if that's at all possible)
>> I'm open to suggestions on the code, the only thing I wasn't happy
>> with was all protocol objects taking a can_read function, but it was
>> the only sane way I could find to make things work. If you can look at
>> that patch (bf9d8a90601d2aad0dfc57f0860196d5589e9718) and provide
>> feedback, I'll be happy to see what I can do about fixing it in short
>> order. I don't think the other ones should be terribly controversial
>> (as far as I can see, anyway), they're mostly just organizing and
>> cleaning up the client code.
> I need to take a closer look at the code but at least two changes
> concern me - the lifetime of GitClient used to be just for one request,

Wow, that was *totally* undocumented. Plus, the class hierarchy
totally implies something different.

Regardless, the concrete client implementations weren't proper
subclasses, and either shouldn't claim to be GitClient subclasses or
should be more cleanly implemented. I opted for the latter, since it
means you can hold a single client for a given repo and pass that
around as the only bit of information required to perform further
operations if needed. The actual important parts of GitClient continue
to exist for only a single request.

> now it's living longer. SSHVendor is deprecated but bzr-git relies on
> it existing so it can override it, e.g. with the Paramiko SSH Vendor.

If that deprecation isn't acceptable I'm fine with changing it -
dependency injection just felt like a significantly cleaner way to go.
Maybe both would be nice?

>> Out of curiosity, why the hurry to get 0.6 out?
> It's already long overdue. I was going to do a release roughly three
> weeks ago but then there were some more bugfixes that were coming up
> that I wanted to wait for. Since I'd like to spend more time discussing
> and testing the GitClient changes I'd prefer to wait with landing those
> changes.
>
> I was doing pretty regular releases for Dulwich earlier and I'd like to
> go back to that. Hopefully that should also make it less important for
> particular changes to make it into this release.
>
> I'll also try to communicate my plans with regards to the releases a bit
> better. I've asked some people in private whether there was anything
> they thought would need to be fixed before 0.6.0, but I should've asked
> here on the list.

Yes please - I'm not terribly comfortable relying on dulwich at the
moment, since its development and release cycle has been fairly opaque
(and things that break hg-git sometimes land without warning).

Also, dulwich@lists doesn't count as public to my mind - almost nobody
can see that, and membership is closed (I'm not even on it). I'd much
prefer either that list was open, or we conduct discussions on a
public list that everyone can see.

>
> Cheers,
>
> Jelmer
>