[patch] refactor check, all_revision_ids

[John Arbash Meinel]

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

Martin Pool wrote:
> This is the diff of some refactorings Robert and I did to make 
> it easier to introduce worm/blob/storage; basically to remove the 
> assumption that it is ever a good idea to retrieve the revision 
> graph of the whole repository.
> 
>  - New methods Branch.check and Repository.check.  
> 
>    This allows for it to be implemented differently depending on the type
>    of branch (consider svn branches) or storage format, though it does not
>    vary at the moment.
> 
>  - Check methods return Result objects that can print themselves, 
>    rather than printing output directly.
> 
>  - Cleaner separation between the consistency constraints of a branch
>    and of a repository.
> 
>  - Repository.all_revision_ids is deprecated
> 
>  - Remove obsolete fileid_involved and fileid_involved_between_revs.
>    (Should be using fileids_altered_by_revision_ids).
> 
> 

...
>      def plan_revisions(self):
> -        repository = self.branch.repository
> -        self.planned_revisions = set(repository.all_revision_ids())
> +        repository = self.repository
> +        self.planned_revisions = set(repository._all_revision_ids())
>          self.progress.clear()
>          inventoried = set(self.inventory_weave.versions())
>          awol = self.planned_revisions - inventoried

I thought the goal was to get rid of all_revision_ids() as accessible
outside of Repository. And as near as I can tell, this doesn't seem to
be factoried as part of the Repository objects.


...

> -            note('%6d revisions missing parents in ancestry', 
> +            note('%6d revisions missing parents in ancestry',

Ah Martin, your trailing whitespace fixes. Very tricky of you.

> -
> -    def fileid_involved_between_revs(self, from_revid, to_revid):
...
> -    def fileid_involved(self, last_revid=None):
...

Nice to see these removed.


>             'deprecated_passed',
>             'warn', 'set_warning_method', 'zero_seven',
>             'zero_eight',
> +           'zero_nine',
>             ]
>  
>  from warnings import warn
> @@ -34,6 +35,7 @@
>  DEPRECATED_PARAMETER = "A deprecated parameter marker."
>  zero_seven = "%s was deprecated in version 0.7."
>  zero_eight = "%s was deprecated in version 0.8."
> +zero_nine = "%s was deprecated in version 0.9."
>  

This seems like it should go in, even if the rest doesn't.

> +
> +    def test_check_branch_report_results(self):
> +        """Checking a branch produces results which can be printed"""
> +        branch = self.make_branch('.')
> +        result = branch.check()
> +        # reports results through logging
> +        result.report_results(verbose=True)
> +        result.report_results(verbose=False)

Shouldn't you check the output?


In general, I like the changes, and I would like to see
'all_revision_ids' deprecated. Though I thought Robert said there was
code that was still using it.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFEkMYWJdeBCYSNAAMRAkVjAKCBuhoRTbJioaVM0pP52ShUcph/1QCfRuWs
axiOf7+nC2rh197i1z/wJy0=
=atVK
-----END PGP SIGNATURE-----