On 20 October 2011 23:31, Emmanuel Bourg <ebo...@apache.org> wrote: > Hi, > > I have a mixed feeling about this release. If I remember well this code > predates Java 5, and despite the amazing work that has been put into to > generify the code, I wonder if it has been sufficiently thought out for a > Java 5 world.
I'm inclined to agree; it would be counter-productive to release version 1.0 and then find that there need to be changes that break the API. I just did a build, and there are a lot of missing @Override markers. Also lots of raw types in the Test classes. If there are pressing reasons to provide a release for testing purposes, then create a snapshot for developers to use. At a pinch, I suppose one could release a 0.n version, and then break the API if necessary in 1.0. But it would need to be made clear that the API was not finalised. > I also made some observations, in no specific order: > > - there are several marker interfaces, but maybe some annotations would make > more sense? > > - couldn't Function, UnaryFunction and BinaryFunction be unified with a > single interface using a vararg parameter? > > - EachElement should be able to work on any Iterable. Also I'm not sure to > understand why its constructor is public. > > - Shouldn't Generator implement Iterable? > > - why are equals, hashCode and toString defined in the Functor interface? > > - why Predicate isn't an extension of Function<Boolean> ? > > - why Procedure isn't an extension of Function<Void> ? > > - Why are constants available through both a static field AND a static > method? For example Identity.INSTANCE and Identify.instance(), or > Constant.TRUE and Constant.truePredicate() ? > > - The Javadoc for Limit states "A predicate that returns true the first n > times it is invoked.", but what happens after? Is it the opposite of Offset? > > - Why aren't Limit and Offset serializable like the other classes in the > core package? > > - Limit and Offset could probably use an AtomicInteger instead of a > synchronized block > > - the site has no example easily accessible, the reader is invited to browse > the JUnit tests. That's not really user friendly. > > - I see IllegalArgumentExceptions thrown for null values, shouldn't this be > changed to throw NullPointerExceptions ? > > - @inheritDoc tags should be removed if no additional description is > provided in the subclasses. This tag is only useful for extending the > description from the method of the super class. > > - what's the point of having equals(T) methods in addition to equals(Object) > ? It clutters the API. > > - CollectionTransformer has a todo stating : "TODO revisit this class..." > > - There are several untested classes in the composite package > > - Most of the equals/hashcode methods are not tested > > > I haven't reviewed everything yet, but that's what I saw at a first glance. > > Emmanuel Bourg > > > > Le 20/10/2011 19:41, Simone Tripodi a écrit : >> >> Hi all guys, >> I'm writing to call for a vote to release apache commons-functor-1.0 >> based on RC1. >> >> The vote will stay open for 72 hours and closes on Sunday 23th, at 5:40pm >> CET. >> >> Many thanks in advance for reviewing, have a nice day! >> All the best, >> Simo >> >> Release notes: >> >> http://people.apache.org/builds/commons/functor/1.0/RC1/RELEASE-NOTES.txt >> >> Tag: >> >> >> https://svn.apache.org/repos/asf/commons/proper/functor/tags/FUNCTOR_1_0_RC1/ >> >> Site: >> >> http://people.apache.org/builds/commons/functor/1.0/RC1/site/ >> >> Binaries: >> >> http://people.apache.org/builds/commons/functor/1.0/RC1/binaries/ >> >> Maven Artifacts (staged on Nexus) >> >> >> https://repository.apache.org/content/repositories/orgapachecommons-086/org/apache/commons/commons-functor/ >> >> [ ] +1 release it >> [ ] +0 go ahead I don't care >> [ ] -1 no, do not release it because... (please explain why) >> >> http://people.apache.org/~simonetripodi/ >> http://simonetripodi.livejournal.com/ >> http://twitter.com/simonetripodi >> http://www.99soft.org/ >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> For additional commands, e-mail: dev-h...@commons.apache.org >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
