Sorry, IS in the 6.x branch. Thanks for merging it
On Mon, Nov 11, 2013 at 8:18 AM, John Sarman <[email protected]> wrote: > As far as the Test failing, I think the workaround code to use jndi first > may have caused the test to fail. So far it seems that all the request > pull 50 is not in the 6 branch. > What forced the need for the workaround? > > John > > > On Mon, Nov 11, 2013 at 8:00 AM, John Sarman <[email protected]> wrote: > >> It is not a forced requirement, just an option for full cdi injection. >> >> >> On Mon, Nov 11, 2013 at 3:50 AM, Emond Papegaaij < >> [email protected]> wrote: >> >>> Hi John, >>> >>> I've just merged the pull request in the wicket-6.x branch (still under >>> experimental). The version still is 0.2-SNAPSHOT, as the versions are >>> automatically increased on release. The reason I've merged the pull >>> request is to give us all a common baseline to discuss. Could you please >>> verify I did not break anything merging it? All testcases but one pass. >>> The >>> failing testcase (CdiConfigurationTest.testMultiAppLoad) tries to access >>> the >>> BeanManager from an unmanaged thread, resulting in an NPE. >>> >>> I've already noticed one aspect I do not like: the requirement to >>> annotate >>> your app with @WicketApp. With a Producer method, it should be possible >>> to use the actual application names, without the requirement to duplicate >>> them on your application class. >>> >>> Best regards, >>> Emond >>> >>> On Sunday 10 November 2013 16:44:28 John Sarman wrote: >>> > Edmond, >>> > On July, I worked vigorously to get to the 0.3 snapshot, which was >>> what I >>> > consider the first beta ready version of the move to cdi1.1. The 0.1 >>> and >>> > 0.2 snapshot was 0.1, getting it to work and learning how to request >>> pull >>> > requests. 0.2 was adding some slight fixes and testing. After that I >>> > realized that I was treating the @ApplicationScoped as same scope that >>> > ThreadContext gives to a Wicket App. That is entirely wrong. So the >>> > previous version only properly supports at most 1 Wicket app, the >>> second >>> > could override the Configuration of the first (not acceptable). In my >>> 0.3 >>> > version, I added the code to prevent that, by using the Wicket app key >>> > generated as the key to the configuration properties for an app. This >>> > allows for multiple Wicket apps to be deployed in a Servlet. However, >>> for >>> > whatever reason, that checkin could not properly merge into the 7 >>> branch. >>> > I have to remedy this even if I just have to copy paste the code, to >>> make >>> > git happy ( I blame myself, not Git). In the meantime, I recommend >>> looking >>> > at my latest (albeit broken) pull request >>> > https://github.com/apache/wicket/pull/50 and port that version. It >>> adds >>> > thorough testing, fixes the multiple deploy issue, reintroduces the >>> auto >>> > Conversation, and extends the ConversationalComponent by introducing >>> the >>> > @Conversational, which by default works the same as the Cdi-1.0 >>> > ConverationalComponent, but also allows the propagation and auto >>> feature to >>> > be modified for an Object that uses the annotation, without affecting >>> the >>> > global defaults set during Configuration. The 0.3 also introduces the >>> > CdiWicketFilter. The CdiWicketFilter allows the configuration >>> settings to >>> > be managed in web.xml. It also instantiates the WicketApplication >>> using >>> > Cdi so that the Application is injected before the init() method. The >>> > changes do not break the original Cdi-1.0, initialization technique, to >>> > support the backwards compatibility. >>> > >>> > John >>> > >>> > >>> > >>> > On Sat, Nov 9, 2013 at 3:29 PM, Emond Papegaaij >>> > >>> > <[email protected]>wrote: >>> > > In wicket 6, this code also still is in experimental. The reason I >>> ported >>> > > it to Wicket 6, was to actually use it. wicket-cdi (the old module), >>> is >>> > > usable with 1.1, but not very optimal. One of the main problems with >>> the >>> > > old implementation is the amount of InjectionTargets created. The >>> annoying >>> > > warnings will probably be fixed in Weld ( >>> > > https://issues.jboss.org/browse/WELD-1547), but the fact remains >>> that >>> > > InjectionTargets are very expensive to create and should be cached. >>> > > Another >>> > > thing I like about the new module is that it actually uses CDI, not >>> just >>> > > makes it available in Wicket. Also, the integration requires a lot >>> less >>> > > code in a container that uses Weld (like Wildfly). >>> > > >>> > > I do agree that the code is far from ready. For example, I don't >>> think >>> > > entire packages should be ignored. Also, I don't like how settings >>> like >>> > > auto-conversations are injected. I do like that CDI is used for >>> that, but >>> > > I'd rather see a configuration object with a @Producer method for all >>> > > settings at once. Having the code in wicket 6 allows me to work on >>> these >>> > > issues. I do not expect our current application to be ported to >>> Wicket >>> 7 >>> > > any time soon, but we are migrating to CDI 1.1 on Wildfly. >>> > > >>> > > Best regards, >>> > > Emond >>> > > >>> > > On Fri, Nov 8, 2013 at 10:10 PM, John Sarman >>> <[email protected]> wrote: >>> > > > +1 removal >>> > > > >>> > > > Never should have been merged into the 6 branch and not the 7 >>> until >>> > > > there >>> > > > is a consensus. >>> > > > >>> > > > >>> > > > On Fri, Nov 8, 2013 at 4:07 PM, Igor Vaynberg >>> <[email protected] >>> > > > >>> > > > >wrote: >>> > > > > not sure why this was merged into 6.x but there are a lot of >>> problems >>> > > > > with this contribution as can be seen here [1]. >>> > > > > >>> > > > > i think this should be removed from at least the release branch. >>> > > > > >>> > > > > -igor >>> > > > > >>> > > > > [1] >>> https://github.com/apache/wicket/pull/50#issuecomment-28063112 >>> >>> >> >
