[MERGE] Stacked knits

Ian Clatworthy ian.clatworthy at internode.on.net
Tue Jul 1 02:49:59 BST 2008


Robert Collins wrote:
> On Fri, 2008-06-27 at 20:51 +1000, Ian Clatworthy wrote:
> 
> Feedback on the queries - I have elided simple to-take actions.

Thanks for the feedback.

bb: tweak

>>> +        basis.calls = []
>>> +        # Subsequent adds do delta.
>> I think those last two lines should be swapped order wise.
> 
> ?

I meant that resetting basis.calls should be the first thing done in
the bit that tests "Subsequent adds do delta".

>>> +        raise KnownFailure("Annotation on stacked knits currently fails.")
>>> +        details = test.annotate(key_basis)
>>> +        self.assertEqual([(key_basis, 'foo\n'), (key_basis, 'bar\n')], details)
>>> +        self.assertEqual([("annotate", key_basis)], basis.calls)
>> I'd like to see those last 3 lines (after the raise) commented out.
> 
> Why? This is what we document as 'The right way' to write a known
> failure - do a test you want to pass, find where it fails, and raise a
> KnownFailure at that point, leaving the rest of the test intact.

I personally don't like seeing unreachable code (e.g. after a raise or return)
like that. If that's our convention, then we might want to make it clearer in
http://doc.bazaar-vcs.org/bzr.dev/en/developer-guide/HACKING.html#known-failures
at some point.

In this case, it's pretty minor anyhow as your follow-on patch addresses this
known failure.

Ian C.



More information about the bazaar mailing list