Code review comment for lp:~jameinel/bzr/repack-missing-inventories-437003

Revision history for this message
Vincent Ladeuil (vila) wrote :

77 + def make_branch_with_disjoint_inventory_and_revision(self):

This test is quite hard to understand (because of the bug it's addressing, not the way you wrote it), I wouldn't mind more comments on the individual steps and the associated assertions. (On a second read, I got it, so you may ignore this remark).

113 + else:
114 + self.fail('Could not find pack containing A\'s inventory')

a comment or a clearer message that THIS-SHOULD-NEVER-HAPPEN will do ;)

136 + (repo, rev_a_pack_name, inv_a_pack_name, rev_c_pack_name
137 + ) = self.make_branch_with_disjoint_inventory_and_revision()

Nice indentation !

Overall: nice TDD there ;)

I'm ok to land this as is.

review: Approve

« Back to merge proposal