[MERGE] Stacked knits

[Robert Collins]

On Tue, 2008-07-01 at 11:49 +1000, Ian Clatworthy wrote:
> 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.

I think the follow on patch illustrates the value of doing what I
did :).

Thanks for the review, I'm acting on the 'comments' mail now and will
land it shortly.

-Rob
-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20080701/2949b4b5/attachment.pgp