[MERGE][RFC] C implementation of PatienceSequenceMatcher

Martin Pool mbp at canonical.com
Mon Sep 3 07:54:00 BST 2007


Martin Pool has voted approve.
Status is now: Semi-approved
Comment:
(tweak) This should be mentioned in NEWS when it merges, along with
typical performance impact.

Since Andrew and John have reviewed this I think it's ok to go in.

+class TestPatienceDiffLib_c(TestPatienceDiffLib):
+
+    _test_needs_features = [CompiledPatienceDiffFeature]
+
+    def setUp(self):
+        super(TestPatienceDiffLib_c, self).setUp()
+        import bzrlib._patiencediff_c
+        self._patiencediff_module = bzrlib._patiencediff_c
+
+

(comment)
This is a place where we could use the new test scenario code
(multiply_tests_from_modules()), which is
only used in inventory_implementations at present.  But there are other
places where we multiply tests using subclassing so this is reasonable.

+# Deprecated, use PatienceSequenceMatcher instead
+KnitSequenceMatcher = patiencediff.PatienceSequenceMatcher

(query) I wonder if this should be a deprecated function instead, so 
that
anyone who tries to construct one will get a warning?  On the other hand
that would totally break anyone who tries to use it for isinstance or 
subclassing.
On the whole just the comment is fine.

(tweak) This should be mentioned as an API change.

I've read through the C code.  I don't feel I could say for sure that it
has no bugs, but it looks reasonable and clear.  I think we can go ahead
and merge it and see what we find out.

You don't need to resubmit for the NEWS changes.

I'm going to merge it now and try some ad hoc testing.

For details, see: 
http://bundlebuggy.aaronbentley.com/request/%3C1186143986.6260.40.camel%40nemo%3E



More information about the bazaar mailing list