fast-reconcile review

Martin Pool mbp at sourcefrog.net
Fri Mar 3 00:58:00 GMT 2006


Just a few doc comments, otherwise +1.

(As I said in person) I am a bit concerned that because this doesn't
insert breadth-first it will cause weaves to increase in size.  It won't
have this effect on knits and if you think it's tolerable it's OK with me.
If it was a problem we could work on a faster breadth-first
reconcile/repack and run that later.

> === modified file 'bzrlib/tests/blackbox/__init__.py'
> --- bzrlib/tests/blackbox/__init__.py	
> +++ bzrlib/tests/blackbox/__init__.py	
> @@ -29,6 +29,8 @@
>                            TestSuite,
>                            TestLoader,
>                            )
> +import bzrlib.ui as ui
> +
>  
>  def test_suite():
>      testmod_names = [
> @@ -47,6 +49,7 @@
>                       'bzrlib.tests.blackbox.test_missing',
>                       'bzrlib.tests.blackbox.test_outside_wt',
>                       'bzrlib.tests.blackbox.test_pull',
> +                     'bzrlib.tests.blackbox.test_reconcile',
>                       'bzrlib.tests.blackbox.test_re_sign',
>                       'bzrlib.tests.blackbox.test_revert',
>                       'bzrlib.tests.blackbox.test_revno',
> @@ -77,3 +80,21 @@
>              return self.run_bzr_captured(args, retcode=retcode)[0]
>          else:
>              return self.run_bzr_captured(args, retcode=retcode)
> +
> +
> +class TestUIFactory(ui.UIFactory):
> +    """A UI Factory which never captures its output.
> +    """

I realize you just moved this class but the docstring seems kind of
strange.

> +class TopoSorter(object):
> +
> +    def __init__(self, graph):
> +        """Topological sorting of a graph.
> +    
> +        :param graph: sequence of pairs of node_name->parent_names_list.
> +                      i.e. [('C', ['B']), ('B', ['A']), ('A', [])]
> +                      For this input the output from the sort or
> +                      iter_topo_order routines will be:
> +                      'A', 'B', 'C'
> +        
> +        node identifiers can be any hashable object, and are typically strings.
> +
> +        The graph is sorted lazily: until you iterate or sort the input is
> +        not processed other than to create an internal representation.
> +
> +        iteration or sortign may raise GraphCycleError if a cycle is present 
> +        in the graph.
> +        """
> +        # a dict of the graph.
> +        self._graph = dict(graph)
> +        ### if debugging:
> +        # self._original_graph = dict(graph)

If you have a graph like [('a', ['b']), ('a', ['c'])] this will lose some
elements; that's fine but it should be noted in the docstring that the
input must be well formed.

> +    def iter_topo_order(self):
> +        """Yield the nodes of a graph in a topological order.
> +    
> +        :param graph: sequence of pairs of node_name->parent_names_list.
> +                      i.e. [('C', ['B']), ('B', ['A']), ('A', [])]
> +                      For this input the following would be yielded:
> +                      'A', 'B', 'C'

This is the sort of thing where a doctest might actually be worthwhile.

Thanks,
Martin

-- 
Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: Digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060303/76f125b5/attachment.pgp 


More information about the bazaar mailing list