Stuart, Thanks for the pointers and explanation on injection points. Will take care of that.
PL On May 31, 10:27 pm, Stuart McCulloch <[email protected]> wrote: > On 1 June 2010 11:12, Pascal-Louis <[email protected]> wrote: > > > Hi all, > > > We've had a long standing grudge against Guice (which we love in all > > other respects!): the inability to have user-defined just-in-time > > bindings. > > > Over the course of the long weekend, I've implemented this feature and > > would love some feedback. Check it out on > >http://eng.kaching.com/2010/05/just-in-time-providers-for-guice.html. > > > I would specifically appreciate feedback from the Guice team on the > > process to integrate this patch into trunk. > > [caveat: just my personal opinion as a fellow patcher...] > > people usually open issues for feature requests / patches: > > http://code.google.com/p/google-guice/issues/list > > as its much easier to track and review than email threads > > I'd also suggest cleaning up the patch so it just contains > the absolute necessary changes for the new feature - the > current patch includes a lot of non-functional / formatting > changes which makes it hard to read and digest > > On a more tactical note, I > > > was unsure about which injection points to use, see the ??? comment in > > the method > > >http://github.com/pascallouisperez/guice-jit-providers/blob/master/sr... > > . > > for ProviderInstanceBindingImpl it should be the injection > points of the provider instance - in the patch this is just a > wrapper around "jitProvider" so it has no injection points, > which means you could simply use: > > ImmutableSet.<InjectionPoint>of() > > this should also remove the need to change InjectionPoint > to handle null superclasses - the InjectionPoint SPI should > only be used on classes (not interfaces) so it always hits > the Object check first - when you used the request key it > might be an interface, which I guess is why you added the > null check to InjectionPoint > > btw, are you arranging for the "jitProviders" to be injected > so that they can also contain @Inject dependencies? > > Best, > > > PL > > > -- > > You received this message because you are subscribed to the Google Groups > > "google-guice-dev" group. > > To post to this group, send email to [email protected]. > > To unsubscribe from this group, send email to > > [email protected]<google-guice-dev%[email protected]> > > . > > For more options, visit this group at > >http://groups.google.com/group/google-guice-dev?hl=en. > > -- > Cheers, Stuart -- You received this message because you are subscribed to the Google Groups "google-guice-dev" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/google-guice-dev?hl=en.
