[bug] Repository.copy() copies inventory first

[Robert Collins]

On Thu, 2006-02-16 at 20:01 -0600, John A Meinel wrote:
> Robert Collins wrote:
> > On Thu, 2006-02-16 at 14:36 -0600, John A Meinel wrote:
> >> Robert Collins wrote:
> >>> +1
> >>> Rob
> >> Do I have a +1 on sign-my-commits? Also on jam-pending.
> >>
> >> Then I can bring them both in.
> > 
> > +
> > +"""Black-box tests for bzr sign-my-commits.
> > +"""
> > +
> > 
> > 
> > Should be
> > 
> > +
> > +"""Black-box tests for bzr sign-my-commits."""
> > +
> > 
> 
> Done
> 
> > 
> > You might like to use TestCaseWithTransport which offers
> > 'self.make_branch_and_tree('.'). This api will use different transport
> > for the branch and repo when requested to, so can give better testing
> > coverage.
> > 
> 
> Done, though ultimately don't you need to set up some sort of adapter
> for it?

It has a default behaviour useful for 'normal' tests.

> > 
> > +             pb = bzrlib.ui.ui_factory.progress_bar()
> > +             copy_all(self.weave_store, destination.weave_store, pb=pb)
> > +             pb.update('copying inventory', 0, 1)
> >               destination.control_weaves.copy_multi(self.control_weaves,
> >                                                    ['inventory'])
> > -             copy_all(self.weave_store, destination.weave_store)
> > -             copy_all(self.revision_store, destination.revision_store)
> > +             copy_all(self.revision_store, destination.revision_store,
> > pb=pb)
> > 
> > I think you've added an 80-char with PEP violation there. Also, doesn't
> > 'destination.control_weaves.copy-multi()' want 'pb=pb' passed to it ?
> > 
> 
> I wasn't planning on passing the inventory copy, because it is only
> copying a single file ('inventory.weave').
> Instead I figured it was better to give a specialized update which made
> it clear what was being copied.

Seems to me that transports like SFTP can provide 
[========           ] 'copy inventory' 150/400

indicators. Inventory.weave tends to get very big, and I'd expect  some
ui feedback during that to be nice.

> > 
> > === modified file 'bzrlib/store/__init__.py'
> > --- bzrlib/store/__init__.py    
> > +++ bzrlib/store/__init__.py    
> > @@ -35,6 +35,7 @@
> >  from bzrlib.trace import mutter
> >  import bzrlib.transport as transport
> >  from bzrlib.transport.local import LocalTransport
> > +import bzrlib.ui
> > 
> > Seems to be an unused import ?
> > 
> > 
> > Other than that, +1.
> > 
> > Rob
> 
> I thought I got all of those. That one got left because store used to
> create a pb in copy_multi, but I thought it was better to be consistent
> and pass one in to all copy functions. In general, I don't like deeply
> nested code to be creating ui objects.
> 
> I would actually prefer not to have ui_factory, since it means the
> library is maintaining state, rather than having ui taken care of by
> callbacks. Though having a factory is better than a lot of other ways.
> 
> Anyway, I made your suggested changes. Is this good enough to be merged?

Yup. +1.

Rob

-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060217/b5ad1bb5/attachment.pgp