dulwich-users team mailing list archive

[David Borowitz <dborowitz@xxxxxxxxxx>]

On Sun, May 30, 2010 at 07:18, Jelmer Vernooij <jelmer@xxxxxxxxx> wrote:

> Hi Dave,
>
> 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.


Will do in the future, unless you have some more specific comments about
these patches.


>  > 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. :-(


I had exactly the same thought when I first factored BaseRepo out of Repo.


>  > 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.
>
> 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.


I wouldn't mind not making this API change and instead working around it in
the places that I need it. That said, here are the reasons I made this
change:
-Dumb HTTP serving was already broken on Windows; it takes does basically
os.path.join(repo.controldir, url_path). Obviously this could be fixed in
web.py instead.
-It made the implementation and tests for put/get_named_file slightly easier
to write.
-It's consistent with what git already does for refs.

I don't think we have the right level of access control to make
get_named_file private/protected--it essentially needs to be what Java calls
package-private, which there's no good way to do in Python.


> > f8d06e0 Add a MemoryRepo that stores everything in memory.
> Nice!
>
> Cheers,
>
> Jelmer
>