Good Morning Mikhail! > I committed those changes as ONAMI-79 [1].
very good, well done! > I also made ConfigurationStateProvider package private and Singleton (it > was unscoped). this sounds something that could have been tracked in JIRA, I suggest you to granularize codebase modifications as well, and track APIs changes with issues on Jira. > Do we want ValidateMethodInterceptor to be public API? Is there any value > in that? I just don't know. > IIRC, if the interceptor is not public, Guice cannot introspect the injection points - do you have spare time to give a test? If that works, then fill an issue on Jira and make it private, no reasons to keep it public. > I can't find a way to assign that issue to myself in Jira - not enough > carma? :) Ah yes, I always forget that Jira groups are not LDAP groups - I'll add you in the right group in a short while. Thanks, have a nice weekend! -Simo http://people.apache.org/~simonetripodi/ http://simonetripodi.livejournal.com/ http://twitter.com/simonetripodi http://www.99soft.org/ On Sun, Feb 10, 2013 at 9:48 AM, Mikhail Mazursky <[email protected]> wrote: > Hello Simone! > > > Some questions: > Do we want ValidateMethodInterceptor to be public API? Is there any value > in that? I just don't know. > > I can't find a way to assign that issue to myself in Jira - not enough > carma? :) > > WDYT? > > [1]: https://issues.apache.org/jira/browse/ONAMI-79 > > > 2013/2/5 Simone Tripodi <[email protected]> > >> > Looks like original authors just prefer setter/field injection. >> >> feel free to speak with me, I am the "guilty" guy ;) >> >> feel free to improve it, fill an issue and assign it to yourself, and >> no reason to provide a patch - we have an SCM wich allow us review the >> code, it sends us emails notification when someone checks in code, so >> we can discuss about codebase modifications. >> >> Thanks a lot in advance, all the best! >> -Simo >> >> http://people.apache.org/~simonetripodi/ >> http://simonetripodi.livejournal.com/ >> http://twitter.com/simonetripodi >> http://www.99soft.org/ >> >> >> On Mon, Feb 4, 2013 at 8:57 AM, Mikhail Mazursky >> <[email protected]> wrote: >> > Hi, Simone! >> > >> > All classes where setters can be replaced with constructor injection. >> Like >> > ValidatorProvider. I'll take a closer look once i have time (probably on >> > the WE) and send patch for review. >> > >> > Looks like original authors just prefer setter/field injection. IMHO it's >> > not the best way to constuct singletons as it lacks enforced >> immutability. >> > Also, it may be not 100% correct under Java memory model. >> > >> > 2013/2/3 Simone Tripodi <[email protected]> >> > >> >> Hi Mikhail! >> >> >> >> thanks a lot for reviewing! Can you specify please the class(es) you >> >> noticed can be improved? >> >> >> >> TIA, all the best, >> >> -Simo >> >> >> >> http://people.apache.org/~simonetripodi/ >> >> http://simonetripodi.livejournal.com/ >> >> http://twitter.com/simonetripodi >> >> http://www.99soft.org/ >> >> >> >> >> >> On Sun, Feb 3, 2013 at 6:08 AM, Mikhail Mazursky >> >> <[email protected]> wrote: >> >> > Hello. >> >> > >> >> > Validation looks good. One thing i would have improved in code is get >> rid >> >> > of setter injection in favour of constructor injection. That would >> made >> >> all >> >> > those classes explicitly immutable and thread safe. >> >> > >> >> > >> >> > 2013/2/2 Simone Tripodi <[email protected]> >> >> > >> >> >> Salut Eric, >> >> >> >> >> >> since you mentioned the validation: did you have the time to have a >> >> >> look at the onami migrated [validation] component? migration should >> be >> >> >> quiet complete, but I'd wait for feedbacks after a discussion before >> >> >> to move it to /trunk. >> >> >> >> >> >> TIA! >> >> >> -Simo >> >> >> >> >> >> http://people.apache.org/~simonetripodi/ >> >> >> http://simonetripodi.livejournal.com/ >> >> >> http://twitter.com/simonetripodi >> >> >> http://www.99soft.org/ >> >> >> >> >> >> >> >> >> On Sat, Feb 2, 2013 at 3:54 PM, Simone Tripodi < >> >> [email protected]> >> >> >> wrote: >> >> >> >> btw, sitebricks for which I have just created a pull-request for >> >> >> validation >> >> >> >> with bval-guice [1] has a dedicated module for convertion [2]. >> >> >> > >> >> >> > cool stuff, very well done, congrats! :) >> >> >> > -Simo >> >> >> > >> >> >> > http://people.apache.org/~simonetripodi/ >> >> >> > http://simonetripodi.livejournal.com/ >> >> >> > http://twitter.com/simonetripodi >> >> >> > http://www.99soft.org/ >> >> >> >> >> >>
