Code review comment for lp:~spiv/bzr/fetch-spec-everything-not-in-other

I think I like the concept of splitting these out into something more like objects.

I don't want to hold this hostage but it seems like this would be a good moment to add a little developer documentation to the architectural guide describing at a high level how fetch works.

+ def get_network_struct(self):
+ start_keys = ' '.join(self._recipe[1])
+ stop_keys = ' '.join(self._recipe[2])
+ count = str(self._recipe[3])
+ return (self._recipe[0], '\n'.join((start_keys, stop_keys, count)))
+

rather than adhoc encoding, could you send it as bencode?

+ medium._remember_remote_is_before((2, 3))

out of scope for this, but concluding it's older than 2.3 because it doesn't support this particular operation is really not accurate.

+ """See InterRepository.searcH_missing_revision_ids()."""

typo.

+ def test_fetch_everything_backwards_compat(self):
+ """Can fetch with EverythingResult even when the server does not have
+ the Repository.get_stream_2.3 verb.
+ """
+ local = self.make_branch('local')
+ builder = self.make_branch_builder('remote')
+ builder.build_commit(message="Commit.")
+ remote_branch_url = self.smart_server.get_url() + 'remote'
+ remote_branch = bzrdir.BzrDir.open(remote_branch_url).open_branch()
+ self.hpss_calls = []
+ local.repository.fetch(remote_branch.repository,
+ fetch_spec=graph.EverythingResult(remote_branch.repository))
+

I don't understand how this tests that the server doesn't have this verb.

=== modified file 'doc/en/release-notes/bzr-2.3.txt'
--- doc/en/release-notes/bzr-2.3.txt 2010-12-14 23:32:28 +0000
+++ doc/en/release-notes/bzr-2.3.txt 2010-12-15 05:36:46 +0000
@@ -158,6 +158,11 @@
   crashes when encountering private bugs (they are just displayed as such).
   (Vincent Ladeuil, #354985)

+* The ``revision_id`` parameter of
+ ``Repository.search_missing_revision_ids`` and
+ ``InterRepository.search_missing_revision_ids`` is deprecated. It is
+ replaced by the ``revision_ids`` parameter. (Andrew Bennetts)
+
 Internals
 *********

spiv said on irc that he's testing this; if it turns out to be faster in practice that should be mentioned too. But I guess this patch alone won't change performance.