I reviewed the site, the Javadoc and the test coverage. Here are my comments:
* The release notes [1] states: "This release is not source or binary compatible with v3.x." I think this might be somewhat confusing, as one could believe the version 4 conflicts with the version 3.x when they can actually coexist together. I would emphasize that this version lives in a new package and is independent of the version 3.x. * The removal of ExtendedProperties is under the "Removed features now supported by the JDK" section. * I didn't see in the release notes the minimum version of Java supported. Is it Java 5 or Java 6? * Why aren't the Closure and Predicate interfaces in the functor package with their implementations? The Equator interface and its implementation DefaultEquator are both in the functor package. This looks like an inconsistency. * org.apache.commons.collections4.Put: I don't quite understand why the return type of put(K, V) is object and not V. What is the object returned? Is it unspecified? I think it would be worth clarifying the Javadoc here. * org.apache.commons.collections4.Get: Why get(Object) and not get(K)? That's odd because entrySet() returns Set<Map.Entry<K,V>>. * Put and Get: There isn't much Javadoc on the methods. * Trie: More documentation on the interface would be nice, so we can quickly understand how it works without following the link to Wikipedia. * CollectionUtils.collate(Iterable, Iterable, boolean) has an extra "/**" in its description. * Why CollectionUtils.collate() instead of CollectionUtils.merge(). The latter sounds more intuitive to me. * PatriciaTrie is a trivial subclass of AbstractPatriciaTrie specifying a KeyAnalyzer. Would that make sense to merge the two classes? * TransformedSortedSet.transformedSortedSet(SortedSet, Transformer) is not tested * IteratorEnumeration, EntrySetMapIterator, AbstractMapIteratorDecorator and AbstractOrderedMapIteratorDecorator are not tested. * Several paths of InstantiateFactory are untested. * UnmodifiableBag.unmodifiableBag() isn't tested in the case of an Unmodifiable bag passed as parameter. * Why is Unmodifiable a marker interface instead of an annotation? If it makes the detection of unmodifiable collection easier I would suggest adding an isModifiable() method in CollectionUtils that would also detect the unmodifiable collections from the JDK. * UnmodifiableSortedBagunmodifiableSortedBag() takes a SortedBag<E> but UnmodifiableBag.unmodifiableBag() takes a Bag<? extends E>, why? * TransformedSortedBag.transformedSortedBag() is not tested. * ComparatorUtils has a very low test coverage. * There are many untested methods in MapUtils (getXXX, getXXXValue, toProperties) Emmanuel Bourg [1] http://people.apache.org/builds/commons/collections/4.0/RC2/release_4_0.html Le 08/11/2013 10:59, Thomas Neidhart a écrit : > Hi, > > I'd like to call a vote for releasing Commons Collections 4.0 based on RC2. > > Changes since RC1: > > * release notes are now also included in the binary distribution > * removed spurious hashCode() method in AbstractPatriciaTrie, the > inherited one from AbstractMap will be used instead > > > Collections 4.0 RC2 is available for review here: > https://dist.apache.org/repos/dist/dev/commons/collections/ > (svn revision 3457) > > Maven artifacts are here: > > > https://repository.apache.org/content/repositories/orgapachecommons-090/org/apache/commons/commons-collections4/4.0/ > > The tag is here: > > > https://svn.apache.org/repos/asf/commons/proper/collections/tags/COLLECTIONS_4_0_RC2/ > (svn revision 1539961) > > Site: > http://people.apache.org/builds/commons/collections/4.0/RC2/ > > Details of changes can be found in the release notes: > > > https://dist.apache.org/repos/dist/dev/commons/collections/RELEASE-NOTES.txt > > http://people.apache.org/builds/commons/collections/4.0/RC2/changes-report.html > > Please review the release candidate and vote. > This vote will close no sooner than 72 hours from now. > > [ ] +1 Release these artifacts > [ ] +0 OK, but... > [ ] -0 OK, but really should fix... > [ ] -1 I oppose this release because... > > Note: the clirr report is generated against 4.0-alpha1 and it is also > highlighted in the release notes that this release is not compatible > with any previous release. > > Note2: there have been reports in the past that some unit tests fail > with certain versions of the IBM Java 6 VM. Some tests have been > disabled when run with a IBM Java 6 VM due to some bugs in the > java.util.TreeMap implementation, but it may still fail for other versions. > > Thank you for your reviews, > > Thomas > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org >
signature.asc
Description: OpenPGP digital signature
