Code review comment for lp:~spiv/bzr/stacking-friendly-revision-history-verb

Revision history for this message
Martin Pool (mbp) wrote :

Does this need a NEWS entry?

Please put the bug url in a comment in the tests to give more context.

The rest of these are general comments are needn't delay merging:

65 + # This figure represent the amount of work to perform this use case. It
66 + # is entirely ok to reduce this number if a test fails due to rpc_count
67 + # being too low. If rpc_count increases, more network roundtrips have
68 + # become necessary for this use case. Please do not adjust this number
69 + # upwards without agreement from bzr's network support maintainers.
70 + self.assertLength(43, self.hpss_calls)

This feels like it should be a specific assertion rather than copy and pasted so many times...

It's unfortunate to hardcode 1.9 in these tests but we should probably scan for them later.

88 + except unstackable_format_errors, e:
89 + raise TestNotApplicable('Format does not support stacking.')

Some people would say you should check at the start of the test whether it promises to support stacking.

Looks good.

review: Approve

« Back to merge proposal