[MERGE][385191] Use the new progress reporting bzrlib API

Jelmer Vernooij jelmer at samba.org
Wed Jun 10 19:58:43 BST 2009


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



More information about the bzr-gtk mailing list