[REVIEW] Progress bars for tree transforms

[Aaron Bentley]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

John A Meinel wrote:
> @@ -1749,7 +1751,8 @@
>          try:
>              conflict_count = merge(other, base, check_clean=(not force),
>                                     merge_type=merge_type,
> reprocess=reprocess,
> -                                   show_base=show_base)
> +                                   show_base=show_base,
> +                                   pb=ui_factory.progress_bar())
> 
> 
> 
> Doesn't this create a default progress bar at first import time, rather
> than creating a new one each time the function is called?

ui_factory.progress_bar is called when you execute cmd_merge.run()

> I think if you are doing this, then:
> pb = None
> if not pb:
>   pb = ui_factory.progress_bar()

The conditional will always evaluate to True, so what's the point?

> But remember, we shouldn't do 'from bzrlib.ui import ui_factory' since
> that creates a local variable, and the way you install a new factory is
> by overriding that variable.

AIUI, imports do not create new variables, they make existing names
visible in another namespace.  The first import would be an exception,
obviously, because the module or package is initialized then.

> I also prefer passing in the progress bar, so that the calling code can
> decide if a progress bar should be displayed, rather than the code
> always creating its own progress bar.

The only caller is run_bzr_catch_errors, and it never specifies whether
progress should be permitted.

> Is this in your 'bzr.ttransform' branch? I would like to pull it so I
> can see the actual progress bar work. Which is something I am looking
> forward to.

No, I did this work on a separate branch.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFD+1rr0F+nu1YWqI0RAlIeAJ0aPuvXqkjW+UykL9a1YyUQLUPmxQCeKxoR
+LT9EtTNISkaObximFv4+E4=
=IcFE
-----END PGP SIGNATURE-----