dulwich-users team mailing list archive

[Augie Fackler <durin42@xxxxxxxxx>]

On Mon, Jun 7, 2010 at 5:32 PM, Jelmer Vernooij <jelmer@xxxxxxxxx> wrote:
> On Mon, 2010-06-07 at 15:59 -0500, Augie Fackler wrote:
>> On May 30, 2010, at 9:18 AM, Jelmer Vernooij wrote:
>> > On Mon, May 24, 2010 at 11:08:56AM -0700, David Borowitz wrote:
>> > > This is quite the grab-bag of changes :). Available as usual at my
>> > > github
>> > > fork:
>> > > http://github.com/dborowitz/dulwich
>> > >
>> > > I think Augie has requested that we do code reviews in some place
>> > > public,
>> > > like this mailing list. If others agree I'm happy to oblige by
>> > > mailing
>> > > patches.
>> > That'd be great - mailed patches are a bit easier to comment on for
>> > me, and this
>> > mailing list seems like the most appropriate place.
>> >
>> > > b9677d0 Add tests for sorted_tree_items and C implementation.
>> > > 186fb3a Fix memory leak in C implementation of sorted_tree_items.
>> > > 0994d03 Clean up file headers.
>> > > 4a42aad Fix copyright in dul-web.
>> > I've merged these, thanks.
>> >
>> > > e146b26 Move named file initilization to BaseRepo.
>> > Thanks, this makes sense.
>> >
>> > We should've really started out by naming BaseRepo Repo and naming
>> > Repo
>> > DiskRepo or something, but it seems hard to change that now without
>> > breaking
>> > everybody's existing code. :-(
>> >
>> > > 9f08973 Use / as separator for named files on all platforms.
>> > I'm not sure about this change - other than the use of info/excludes
>> > we don't
>> > actually appear to use / in calls to {get,put}_named_file so it
>> > seems like a
>> > silly thing to break the API for.
>> Is it an API break then? It also means that you can always request
>> 'info/excludes' without respect to the platform you're on.
> It changes the behaviour of this particular call on some platforms
> (specifying info\excludes on Windows will break), so users will have to
> change their callsites.
>
>> > Alternatively, perhaps we can make get_named_file private/protected?
>> > I'd be
>> > less hesitant about making this sort of change if we do that.
>
>> -1, it feels like clients of Repo will need to load files within .git
>> and get_named_file feels like the correct call for this.
> Do they need to do that directly though, and for what sort of
> functionality? I'd rather see higher-level function calls on Repo, in
> particular because that makes it easier to e.g. implement the Repo API
> on top of other databases with a different on-disk format.

It seems reasonable that we'll need access to (for example)
info/excludes so we can construct the appropriate matcher for
Mercurial, Bazaar or whatever else. It seems kind of odd to my mind to
have a function per special file on the repo object.

>
> Cheers,
>
> Jelmer
>
>