[MERGE] Give commit a progress bar
robertc at robertcollins.net
Fri Apr 21 08:00:36 BST 2006
On Fri, 2006-04-21 at 16:49 +1000, Martin Pool wrote:
> I think this is useful but I'd really like at this point to freeze out
> any non-critical bugs so that we can get 0.8 out. So please add this to
> a wishlist bug and keep the branch until next week when we can reopen
> for general development.
> So +1 with comments, but not right now.
> On 21 Apr 2006, Robert Collins <robertc at robertcollins.net> wrote:
> > --
> > GPG key available at: <http://www.robertcollins.net/keys.txt>.
> > === added file 'b/bzrlib/tests/workingtree_implementations/test_commit.py'
> > +class CapturingUIFactory(ui.UIFactory):
> > + """A UI Factory for testing - capture the updates made through it."""
> Perhaps this should move (along with its selftests) to a more generic
> location, to encourage other test code to use it?
> > === modified file 'a/bzrlib/commit.py'
> > --- a/bzrlib/commit.py
> > +++ b/bzrlib/commit.py
> > @@ -245,7 +245,12 @@
> > self.reporter = reporter
> > self.work_tree.lock_write()
> > + self.pb = bzrlib.ui.ui_factory.nested_progress_bar()
> > try:
> > + # Cannot commit with conflicts present.
> > + if len(self.work_tree.conflicts())>0:
> > + raise ConflictsInTree
> > +
> > # setup the bound branch variables as needed.
> > self._check_bound_branch()
> I'm curious - why did you move the conflict detection from below?
Bug #5798: 'conflicts in tree' check happens after commit message
If you type in a careful email, and then get a conflict warning, it can
piss one off. I was in the area.. shrug.
> if found_exception is not None:
> > + # dont do a plan raise, because the last exception may have been
> > + # trashed, e is our sure-to-work exception even though it loses the
> > + # full traceback. XXX: RBC 20060421 perhaps we could check the
> > + # exc_info and if its the same one do a plain raise otherwise
> > + # 'raise e' as we do now.
> > + raise e
> I see this just factors out the existing code but it feels quite
> strange. Shouldn't we just have multiple try/finally blocks, and split
> out the code if that makes it too deeply nested?
Well, its likely that it will grow further, and we have multiple places
this will be needed eventually - for instance in doCleanups [or similar]
in TestCase.tearDown(), in the multiple-unlock actions of InterFOO
classes, etc. Seems to me we should figure out the right idiom in once
place and once that is done move it to a class representing cleanups
> > + def _update(self):
> > + """Emit an update to the progress bar."""
> > + self.pb.update("Committing", self.pb_count, self.pb_total)
> > + self.pb_count += 1
> > +
> > def _select_entry(self, new_ie):
> > """Make new_ie be considered for committing."""
> > ie = new_ie.copy()
> I'd much prefer _update_pb or some such, _update is a bit generic.
> > + def clear_term(self):
> > + """See progress.ProgressBar.clear_term()."""
> > +
> > def finished(self):
> > """See progress.ProgressBar.finished()."""
> I (still) dislike these docstrings; they use up memory and hide the real
> docstring without adding anything useful. (I know we talked about it
> before, no action is required for this commit.) I would prefer if they
> could just be comments.
Do we have a bug open on the solution we came up with? Or a spec? Either
of those would be good - as a QA aspect of the code base its something I
might get around to post dapper :)
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 191 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060421/18070183/attachment.pgp
More information about the bazaar