2013/2/10 Simone Tripodi <[email protected]> > Good Morning Mikhail! > > It's 16:37 here =)
> > 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. > > Ok, understood. > > 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. > > Tests pass with interceptor declared package private. I applied changes as ONAMI-80. > > 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. > > Works fine now, thanks! > Thanks, have a nice weekend! > Let it be nice WE for you too! > -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/ > >> >> >> > >> >> > >> >
