Ok, i'll go ahead and commit ...

2016-05-19 21:30 GMT+02:00 David Jencks <[email protected]>:

> Now I did :-)
>
> I’m basically OK with this.  I have a couple minor questions that I think
> we can resolve later unless you want to respond now.
>
> 1.  I  think Coercions should stay in the same package as Annotations
> since this is basically an implementation of the forthcoming object
> converter spec.  I didn’t look to see if there is an appropriate spec
> method for a simple object to integer conversion which I think is the only
> non Annotations use of Coercions.  Maybe we should copy the current object
> converter interface(s) and use them.
>

There is one usage, which I think was the reason why I did not move
Coercions.
It's used by DependencyManager#getMinimumCardinality


> 2. I don’t quite see why you have more or less changed the methods taking
> a RawParemeter to something else.  Could you explain?  Having done this
> shouldn’t we make the old methods less public?
>

You mean the various invoke() methods in ReferenceMethod / ComponentMethod.
I think I realised some classes were really only instantiated from the new
inject package, so it made sense to move them along and abstract a bit
more.  If you think it's less clear, we can change back that bit.

>
> 3. I think there might be one or two new inner classes that perhaps should
> be static classes?  I see one in ComponentRegistry.
>

The one createComponentHolder() I suppose ? yes, it definitely should be
static.


> I guess now I’d like you to commit this soon since I’m changing how the
> circular reference tracking works a little bit and that’s going to overlap
> with this.
>
> IIUC you did this in large part so you can reuse some bits elsewhere.  How
> are we going to keep appropriate isolation going forwards? (i.e. so I don’t
> break your cdi stuff …)
>

That's definitely a good point.  I'll try to setup a test to ensure that
the needed packages can be used in isolation.


>
> thanks!
> david jencks
>
> > On May 19, 2016, at 4:21 AM, Guillaume Nodet <[email protected]> wrote:
> >
> > Did you had a chance to look at it ?
> >
> > Guillaume
> >
> > Le mardi 3 mai 2016, Guillaume Nodet <[email protected] <mailto:
> [email protected]>> a écrit :
> >
> >>
> >>
> >> 2016-05-03 19:12 GMT+02:00 David Jencks <[email protected]
> <mailto:[email protected]>
> >> <javascript:_e(%7B%7D,'cvml','[email protected] <mailto:
> [email protected]>');>>:
> >>
> >>> Hi Guillaume,
> >>>
> >>> Somehow I missed the  issue being filed.  I’d appreciate it if you
> could
> >>> hold off committing this until I have affirmatively reviewed the
> proposal.
> >>> I’ll try to get to it soon, but can’t promise.
> >>>
> >>
> >> There's no rush. I'm currently working on a copy of it, but obviously,
> I'd
> >> rather use the official SCR code at some point.
> >>
> >> Thx
> >>
> >>
> >>
> >>>
> >>> thanks
> >>> david jencks
> >>>
> >>>> On May 3, 2016, at 7:54 AM, Guillaume Nodet <[email protected]
> >>> <javascript:_e(%7B%7D,'cvml','[email protected] <mailto:
> [email protected]>');>> wrote:
> >>>>
> >>>> I've raised FELIX-5243 about refactoring some SCR code but haven't had
> >>> any
> >>>> feedback.
> >>>> The commits are visible at:
> >>>> https://github.com/gnodet/felix/commits/FELIX-5243
> >>>> Unless someone complains, and given it's only refactoring and should
> not
> >>>> alter the code itself (and all tests pass), I'll push those changes
> >>> later
> >>>> this week.
> >>>>
> >>>> Cheers,
> >>>> Guillaume
> >>>
> >>>
> >>
> >>
> >> --
> >> ------------------------
> >> Guillaume Nodet
> >> ------------------------
> >> Red Hat, Open Source Integration
> >>
> >> Email: [email protected] <mailto:[email protected]>
> >> <javascript:_e(%7B%7D,'cvml','[email protected] <mailto:
> [email protected]>');>
> >> Web: http://fusesource.com <http://fusesource.com/>
> >> Blog: http://gnodet.blogspot.com/ <http://gnodet.blogspot.com/>
> >>
> >>
> >
> > --
> > ------------------------
> > Guillaume Nodet
> > ------------------------
> > Red Hat, Open Source Integration
> >
> > Email: [email protected] <mailto:[email protected]>
> > Web: http://fusesource.com <http://fusesource.com/>
> > Blog: http://gnodet.blogspot.com/ <http://gnodet.blogspot.com/>
>



-- 
------------------------
Guillaume Nodet
------------------------
Red Hat, Open Source Integration

Email: [email protected]
Web: http://fusesource.com
Blog: http://gnodet.blogspot.com/

Reply via email to