← Back to team overview

dulwich-users team mailing list archive

Re: [PATCH 0/9] Small features and bugfixes

 

On Mon, Jul 26, 2010 at 12:22, Jelmer Vernooij <jelmer@xxxxxxxxx> wrote:

> On Mon, 2010-07-26 at 12:14 -0700, David Borowitz wrote:
> > On Mon, Jul 26, 2010 at 11:44, Jelmer Vernooij <jelmer@xxxxxxxxx>
> > wrote:
> >         On Mon, 2010-07-26 at 09:09 -0700, dborowitz@xxxxxxxxxx wrote:
> >         > a52bcb5 object_store: Make iter_tree_contents depth-first.
> >         > 5e2302e object_store: Include subtrees in iteration.
> >
> >         I'm waiting a bit with these because I want to have a closer
> >         look.
> >
> >
> > Is there something in particular that concerns you, or is it just that
> > the implementation is not as trivial as the rest of the patch series?
> Mainly the fact that they're not as trivial. There isn't anything in
> particular that concerns me, though the fact that they contain API
> changes makes me a bit more cautious than usual (in general, but also
> because of my own code that uses these methods).
>

FWIW, the first change should preserve the output set, just providing a
guaranteed order. (Hopefully you weren't depending on any ordering of the
output before, since elements are popped from sets in arbitrary order.) The
second change can change behavior, but the default for the new kwarg matches
the old value.

In retrospect, a useful thing to do would have been to add the test before
modifying the method, so we could verify that the behavior (other than
ordering) hasn't changed. Let me know if you want me to do that.


> Cheers,
>
> Jelmer
>
>

References