On 30/07/2012 09:36, Liviu Tudor wrote: > Thanks very much for pointing me in the right direction, Mark! > As with all development tasks, this, however, opened up new questions -- > not entirely sure if these are best discussed on this list or whether > occasionally I should open up a JIRA ticket for this so more than > welcoming suggestions here.
Jira is not the place for discussions. That is best done on the dev list. Once we have an agreed direction then we can open a Jira to track the work. > * on the [pool2] -- I haven't investigated the eviction mechanism in > detail there, but I understand the need for a List and that's fine. My > original idea was to provide a Set with all the keys for the > KeyedObjectPool interface, as there is no need for ordering of the keys > here (same as in Map.keySet) however, I can just as well have a List and > provide a getKeys() in the KeyedObjectPool interface -- it will be used > the same ultimately for my idea, which in fact needs just a collection > (Set was preferred due to the fact that Map provides access to the keys > via a Set and the fact that keys are unique obviously). So, in the light > of that, which would be better to have in KeyedObjectPool interface (if > any): > 1/ Set<K> keySet(); //returns a Set with all the keys in the pool > 2/ List<K> getKeys(); //this is already implemented part of the Mbean > implementation in GenericKeyedObjectPool > 3/ Collection<K> getKeys();//this allows us to customise it in > subclasses, so we can have List in GenericKeyedObjectPool but perhaps > other implementation can return Set if needed Since the keys are a Set, that seems like the most appropriate option. I'd be happy changing the existing method to use Set. We should think about switching the internal list as well, e.g. to a LinkedHashSet (would need to check behaviour was unchanged). > * Looking at [pool-1.6] -- there are a few problems with the code on that > branch, not sure if this was abandoned or not? Absolutely not. > The layout of the project > is a bit non-maven, but that is fine. Checkstyle throws a bunch of errors Checkstyle 'errors' are not bugs. Cleaner code is nice but not essential. > due to misconfiguration in pom.xml (which I have easily fixed). Also there > are a bunch of FindBungs which I have fixed too. Based on previous experience they may or may not be valid. > There are a few unit > tests failing which I am looking at while also trying to slot in my actual > commit (as per below) :) That sounds like your setup isn't quite right. The unit tests should always pass and did on my machine the last time I touched the 1.x code. There are also regular CI builds that check this. > So, what are the best practices here -- separate these into a bunch of > JIRA issues/patches : > 1/ checkstyle fixes > 2/ findbugs fixes > 3/ unit test fixes > 4/ my component > Or just put everything into one patch? One issue per Jira please. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
