Code review comment for lp:~jtv/launchpad/bug-884649-branch-4

Hi Jeroen,

Lots of optimization here -- I know that warms your innards.

* I'm a little confused by find_live_source_version and find_live_binary_version_pass_1. Both take an iterable called 'sorted_pubs'. In the former the "latest" is element [0] and in the later it is [-1] even though the comments say they are both sorted in descending order. Is that notion of "latest" really different?

* "This class looks up whether there are any." Could you complete that thought?

* I like your ArchSpecificPublicationsCache. I'm a bit (paranoidly) worried about the fragility of the order of elements returned by getKey and those expected by _lookUp ... but can't think of anything smart to suggest.

In find_live_binary_version_pass_2 how long do you expect the sorted_pubs list to be? Would it be reasonable to use set-based methods to determine arch_indep_pubs after you already figured out arch_specific_pubs? BTW, I should pay more attention to itertools -- I learned about ifilter* from this review. Thanks!

* In test_hasArchSpecificPublications_is_consistent_and_correct, I don't understand the value of testing each entry in the cache twice.

Otherwise it looks good. Thanks.