[MERGE] bzr check detects unreferenced revisions

[Aaron Bentley]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andrew Bennetts wrote:
> BB:tweak
> 
> I like this, I just would like it to be a little clearer, especially in
> communicating the intent of the test.  Also, you don't don't see to report all
> the problems you catch.  But basically I'm happy with this change.

I've made a few more changes than you mentioned, so I'm resubmitting.

The main thing is that knit1 repositories are supposed to lack knits for
TREE_ROOT, but these tests would erroneously report this as a problem.

> Aaron Bentley wrote:
> [...]
>> This is my output for bzr:
>>
>>  15566 revisions
>>  35221 unique file texts
>> 5213198 repeated file texts
>> vvv
>>   2450 unreferenced file texts
>> ^^^
> 
> It'd be good to tell users what this means, and what to do about it.  But this
> is a good incremental step towards that.

I documented all the output values, and re-ordered them to group them
more logically.


>> +class TestCheckFileVersions(repository_implementations.TestCaseWithRepository):
>> +
>> +    def create_damaged_repository(self):
> 
> It would be good to have a brief docstring on this method describing the way the
> repository is damaged.  e.g.:

Done.

> Hmm, you seem to be testing several different behaviours here, without clearly
> explaining the intent of any of them.  Maybe break this up in to multiple test
> methods, each with a docstring explicitly describing on the condition you want
> to test.

I've broken them up into multiple test methods.  I think they're self-
documenting now.

> Hmm, so nothing is reported for missing-file or missing-version?  That doesn't
> seem right.

Okay, I've added them to the output.

> Nitpick: sentences should finish with a full stop.

Alright

> 
>> +        Iterates through pairs of (revision_id, result_dict).
>> +        """
> 
> I don't understand what this means.  Is it describing the return value, the
> implementation, or something else?  What is “result_dict”?  What is the return
> value.

Fixed.  (an earlier version was an iterator, but that didn't fit into
"check" very well).

> Ideally this docstring would have “:param revision_tree:” and “:returns:”
> sections.

I can't think of anything intelligent to say about the revision_tree
parameter.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGzZ5R0F+nu1YWqI0RArNJAJ9b4eGPZN+qZGxLg4EFxaVATfwLKQCffo7+
hr9okThzOb20SVqJpau8zT8=
=7/f0
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: check-file-versions.patch
Type: text/x-patch
Size: 24050 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070823/b1530056/attachment.bin