Responses inline.
2014-05-25 15:03 GMT+02:00 Thomas Vandahl <t...@apache.org>: > Hi Romain, > > On 24.05.14 20:16, Romain Manni-Bucau wrote: > > basically I respected all remarks, only pendings ones are: > > > > 1) modules > > Ok, right now, I see the following modules > - commons-jcs-core > - commons-jcs-jcache > - commons-jcs-jcache-extras > - commons-jcs-jcache-openjpa > - commons-jcs-tck-tests > > I don't know anything about "extras" and "openjpa". What is the purpose > of these modules? Do you plan to introduce others? > Extras is a set of useful basic classes for JCache usage. OpenJPA is JCache support for L2 cache in openjpa. The goal is to make what we propose with jcache module as usable as possible and avoid glue code as much as possible in application code. > > > 2) I removed Serializable from *API* of auxilary caches since it prevent > a > > lot of things like using it only in memory on not serializable objects > for > > instance. The constraint is still on the impl which need it but not more > in > > the API > > I understand your comment about different types of serializers (json, > xml and such). On the other hand, I prefer type checks at compile time. > The mixture that exists now creates more problems than it solves. My > suggestion would be to remove the Serializable constraint altogether, > but then what? The explicit declaration makes sure that no > non-serializable objects can be put into the cache, so all levels "below > the surface" can rely on this behavior. If I don't have this > restriction, error handling must be done on the serializer level and > exceptions bubbled up to the API. Would that make sense? > > In any case the modifications of the API must be documented, both in > src/changes/changes.xml and the files below xdocs. > > I was not that happy to do so but it made default usage really easier and opened several possibilities (L2 cache wouldn't have been seriousy possible otherwise since often entities are not serializable for instance). Ok about changes.xml, didnt see it was used, Should be updated. > > 3) I removed "Seconds" suffix in configuration since for JCache we need > it > > to be millisecond. Default is still in second (without the suffix) but we > > need this update otherwise we can't pass JCache tests > > I know that the usage of seconds vs. milliseconds is not intuitive > within JCS. Please make sure that your changes improve this situation > and not make it worse. This change has a big impact on existing users so > please update the documentation! > > Should be done. I just removed Seconds prefix since user shouldn't see something else than seconds by default. We can keep TimeFactorForMilliseconds an "internal" configuration for now IMHO. > > @Thomas: anything I forgotten, thinks that's all now? > > Please document all the changes you've done in src/changes/changes.xml I > tried to catch up with some important features but lost track. > > And please consider reacting to Phils comment on the removal of > synchronized keywords. It took years to pinpoint the race issues in the > JCS code. I don't claim the current situation to be perfect but at least > it does not cause data corruption. I want to go forward, not backward. > > I want to get some momentum to the project as I am currently very time > constrained. That's why I wanted to invite you to the Commons community. > I hope we can get to some consensus by discussing important changes > *before* committing them - even if it takes a few days. I will do my > best to comment, review and contribute and I would expect from you to do > the same. > > We can create a branch from trunk (to keep code) and go back to a revision which is ok for you if you want to discuss commits. > Bye, Thomas. > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > >
