Hi Vincent, Vincent Ladeuil wrote: > This patch uses the new progress reporting API available in bzr > since 1.12 :-P > > The nice thing is that bzr viz now shows a lot more than before > (or at least that what I remember :-) > As far as I can tell nothing has changed. > I'm pretty sure the patch is conservative but I'd appreciate > feedback on any edge case (I mostly tested viz). > Thanks for working on this, it was indeed very much overdue.
There seem to be a lot of unrelated whitespace fixes, they make the patch a bit harder to read. The patch seems ok, in general but there's some FIXME's that don't seem right: > + # FIXME: Why is the following needed ? Are there really cases where > we > + # use a TreeView without installing our own ui ? --vila 20090610 > + if getattr(ui.ui_factory, "set_progress_bar_widget", None) is not > None: > + ui.ui_factory.set_progress_bar_widget(loading_msg_widget) This can happen if somebody else imports bzr-gtk widgets but doesn't change the UI. > + # FIXME: The following seems to be there to provide a default for > cases > + # where set_progress_bar_widget() is not called explicitely. It will > be > + # better to call it explicitely and get rid of that default. (I'm not > + # even sure it really needed now :-/ -- vila 20090610. We rely on this in a lot of places, and we can't get rid of it in the near future, and perhaps never. Progress bars can be created by pretty much any call to bzrlib. If you for example run "bzr gannotate" in a subversion working copy you get a window-based progress bar. > def clear(self): > self.pb.clear() > + # FIXME: destroy() ? Really ? -- vila 20090610 > self.destroy() This is necessary because otherwise the progress bar window stays around, visible to the user. Some ad-hoc testing shows me that the progress windows now stay around frozen rather than being hidden properly. There's some bits of the network activity still being printed to stdout. It would be nice to show this information in the progress bars, or alternatively we could just hide it for now. bb:resubmit > The intent is to land that patch ASAP as one can't use bzr.dev > and bzr-gtk trunk anymore. > > bzr-1.16 is pretty close, it would be nice to release > bzr-gtk-0.9.6 soon (hint [1] :) > > > [1]: Looks like we can settle on bug > https://bugs.edge.launchpad.net/bzr-gtk/+bug/377476 now, yes ? :-D > IMHO that bug report should be closed as invalid for bzr-gtk as it is a problem in seahorse. What do you think? Cheers, Jelmer -- bzr-gtk mailing list [email protected] Modify settings or unsubscribe at: https://lists.canonical.com/mailman/listinfo/bzr-gtk
