← Back to team overview

dulwich-users team mailing list archive

Re: [PATCH 2/4] MissingObjectFinder: minor cleanup: 80 chars, others.

 

On Sun, 2010-08-22 at 20:06 -0700, David Borowitz wrote:
> On Sun, Aug 22, 2010 at 10:02, Jelmer Vernooij <jelmer@xxxxxxxxx>
> wrote:
>         
>         On Wed, 2010-08-18 at 09:41 -0700, dborowitz@xxxxxxxxxx wrote:
>         > From: Dave Borowitz <dborowitz@xxxxxxxxxx>
>         >
>         > Change-Id: I06fe864a6a89d1b77cffd13aac790c15b64224ff
>         > ---
>         >  NEWS                    |    2 ++
>         >  dulwich/object_store.py |   13 +++++++++----
>         >  2 files changed, 11 insertions(+), 4 deletions(-)
>         >
>         > diff --git a/NEWS b/NEWS
>         > index 14cb030..75fe560 100644
>         > --- a/NEWS
>         > +++ b/NEWS
>         > @@ -50,6 +50,8 @@
>         >    * Use real in-memory objects rather than stubs for server
>         tests.
>         >      (Dave Borowitz)
>         >
>         > +  * Clean up MissingObjectFinder. (Dave Borowitz)
>         > +
>         >   API CHANGES
>         >
>         >    * ObjectStore.iter_tree_contents now walks contents in
>         depth-first, sorted
>         > diff --git a/dulwich/object_store.py
>         b/dulwich/object_store.py
>         > index 61192d7..162f102 100644
>         > --- a/dulwich/object_store.py
>         > +++ b/dulwich/object_store.py
>         > @@ -711,8 +711,10 @@ class MissingObjectFinder(object):
>         >
>         >      def __init__(self, object_store, haves, wants,
>         progress=None,
>         >                   get_tagged=None):
>         > -        self.sha_done = set(haves)
>         > -        self.objects_to_send = set([(w, None, False) for w
>         in wants if w not in haves])
>         > +        haves = set(haves)
>         > +        self.sha_done = haves
>         
>         I have concerns about setting sha_done to haves here without
>         copying it.
>         It means we'll end up modifying the set that is being passed
>         in by the
>         caller whereas we previously werent.
> 
> 
> The previous line (haves = set(haves)) still copies it. I just stored
> it in a local to make the list comprehension faster.
My bad, sorry. Clearly I should not do any code review after midnight. 

Merged, thanks.

Cheers,

Jelmer




Follow ups

References