Change Version::max to take an Iterator

[klnusbaum]

Version::max takes an IntoIterator as it's argument. This seems overly restrictive. If I want to use the max function, I now have to pass ownership of what ever data-structure I'm using into the max function (either that, or clone it and then pass the clone into max). What if I still want to keep using that data-structure? I was taking a stab at #1058 and ran into this exact case. I have a vector of versions that I need to do multiple operations on (get all the version ids in the vector, get the length of the vector, as well as get the max version in the vector). It seemed weird that I had to give up ownership of the vector to get it's max version.

Instead, let's just have Version::max take an Iterator instead. This is more permissive and actually doesn't break any existing code.

[sgrif]

This is actually more restrictive than what was there before. All types which implement Iterator implement IntoIterator, but the reverse is not true.

You're still passing ownership, the only difference is now the thing has to already be an iterator rather than a thing which can be converted to one. Any function which operates on iterators will need to take ownership (well technically it only needs to take &mut T where T: Iterator, but assuming the function actually iterates, the iterator will be left in a useless state afterwards, which is the same as taking ownership for all intents and purposes).

Even putting that aside, I'd prefer not to arbitrarily change the signatures of functions without an actual reason to do so (e.g. some call that requires this change in order to work).

[klnusbaum]

@sgrif I see now. I was thinking about this earlier and wondering something along the lines of what you pointed out. I'm a bit of a Rust noob so I appreciate the explanation. I'm still trying to wrap my head around all the various permutations of iterators. This was helpful :)