Hi IndexReader.listCommits returns a Collection, which not only does not define and guarantee any ordering between the commits, but also makes it very hard to know what is the latest commit. And we have some inconsistency, IMO, in how we return the commits today, in the different API:
IndexDeletionPolicy receives a List<IndexCommit> and not Collection. Furthermore, it relies on the commits being ordered oldest -> latest, and it's somewhat documented. listCommits, to my surprise, returns the latest commit as the first element of the Collection, and the rest of the commits are unsorted. Having seen IDP relying on the commits to be sorted, I thought that listCommits sorts them too, but was surprised to find out that the latest one appears first. The implementation already returns an ArrayList, and I wonder why do we need the Collection abstraction? Why not returning a List, or even a LinkedList, or an array - anything that entails ordering between them. And preferably something which will allow me to access first/last really quickly, and direct access can be a bonus for all I care. And we should also document the ordering they are returned, and better make it consistent w/ IDP's assumptions. It's a static method, so no one can break by extending it. And committing to some ordering should be problematic - we already do that with IDPs. The only problem is that we cannot change the signature of the method, so I'm ok w/ either break back-compat and document, or deprecate and create a getCommits (or a better name). While we're at it, it'd be great if we can consolidate ReaderCommit and CommitPoint. The latter implements Comparable and supports deletion of IndexCommits, while the former doesn't do both. Is there any reason why it cannot support delete()? What do you think? Shai
