Code review comment for lp://staging/~jameinel/bzr/2.4-cheaper-iter-entries-by-dir

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

On 3/20/2011 12:18 PM, Vincent Ladeuil wrote:
> Review: Needs Information
> 29 + if from_dir is None and specific_file_ids is None:
> 30 + # They are iterating from the root, and have not specified any
> 31 + # specific entries to look at. All current callers fully consume the
> 32 + # iterator, so we can safely assume we are accessing all entries
> 33 + self._preload_cache()
> 34 if from_dir is None:
> 35 if self.root is None:
> 36 return
>
> You're pre-loading the cache even when from_dir and self.root are None, is there nothing to preload in this case or does it mean the shortcut is now ineffective and what are the consequences then ?
>

If self.root is None, there shouldn't be anything to preload, because if
you have no root, you better not have any children of root.

That said, I'm not 100% sure that CHKInventory can exist without a root.
If you had an object like that, it would probably have to be an
Inventory object.

Now, iter_entries_by_dir() exists only on CommonInventory, so it is
shared between both types. However the base implementation of _preload
is also a no-op.

So if you have a tree with no root, then preload is a no-op (one way or
the other).

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

iEYEARECAAYFAk2GTGAACgkQJdeBCYSNAANO1ACfWVNV/NeV9PqMmB7Gh6Hl0sxU
JckAoNFKKICCU0Zl6ku0VfGYSzPUHzWL
=KmmW
-----END PGP SIGNATURE-----