[MERGE] Refactor fetch: add get_data_about_revision_ids to repository, and other changes.

[John Arbash Meinel]

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

Andrew Bennetts wrote:
> Andrew Bennetts wrote:
> [...]
>> This particular bundle rearranges fetch.py a little.  Most interestingly, it
>> moves some of the logic out of fetch.py, and moves it to a new method on
>> Repository, get_data_about_revision_ids(revision_ids).
> 
> I've addressed all the review comments, but here's a new bundle.  The part I
> want feedback on is the new method name, and its description:
> item_keys_introduced_by(revision_ids).
> 
> I discussed this over the phone with Robert, and the idea behind the new term
> “item keys” is that the things returned by this function are basically keys to
> the various kinds of repository data — this is related to Robert's work on a
> pack file format.
> 
> I'd appreciate any feedback on the best way to describe this so that the
> function's purpose is adequately clear (even though it's only used in one place
> at the moment).  Here's what I have at the moment:
> 
>     def item_keys_introduced_by(self, revision_ids, _files_pb=None):
>         """Get an iterable listing the keys of all the data introduced by a set
>         of revision IDs.
> 
>         The keys will be ordered so that the corresponding items can be safely
>         fetched and inserted in that order.
> 
>         :returns: An iterable producing tuples of (knit-kind, file-id,
>             versions).  knit-kind is one of 'file', 'inventory', 'signatures',
>             'revisions'.  file-id is None unless knit-kind is 'file'.
>         """
> 
> I'm hesistant to go into too much detail in this docstring... it seems to me
> that as Robert's work lands we'll probably have better places for describing the
> concepts than here.
> 
> I've attached the complete bundle for completeness sake, but it's basically the
> same as before modulo the bits changed due to review comments.
> 
> Robert has suggested we might want to deprecate fileids_altered_by_revision_ids
> as a public method, replacing it with this function.  (Callers can just filter
> out the non-file information).  This seems like a good idea to me, Repository
> already has a very large public interface and it would be good to shrink it
> where we can.  But that's a change for a different release cycle!
> 
> All that said, it's not a big deal to improve a docstring or rename an
> little-used method later, so I'm not too concerned about this.  I just want to
> get another opinion or two as a sanity check.
> 
> -Andrew.

I didn't read any of the code, but I think your comments here are very
clear. And that it is significantly better to have a single api than 2
that do almost the same thing. (Arguably we could just turn
'file_ids_altered_by' into calling item_keys_introduced_by and filtering
for you.) If we have that defined at the topmost Repository object, then
subclasses don't have to reimplement it (and neither do callers).

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

iD8DBQFGudisJdeBCYSNAAMRAlmWAJ9jD1iaThX61JIiB1EtU+J+LJ1BoACgk3II
1CNtqkp/3mtiiBCy4BX1iKE=
=6JdC
-----END PGP SIGNATURE-----