Kabir has provided me feedback on this in private. He thinks that, overall, the
integration is good to be ported to trunk. I'll post here the points he addressed in
our private chat. These are the points that need to reviewed:
1. The field ClassPool classPool of Instrumentor should be of type AOPClassPool.
2. I should see if I can find a workaround for checks such as the one below:
if (this.childFirstLookup != delegate.childFirstLookup)
| {
| delegate.childFirstLookup = this.childFirstLookup;
| }
This check is there because childFirstLookup is a public field of ClassPool. AOPClassPool
uses ClassPool as a delegate, so it can delegate to any classpool of jboss-classpool
project. However, to force AOPClassPool and ClassPool to work consistently, I added these
checks. This will be reviewed, but we are both afraid that maybe there is no solution for
this.
3. Should ClassPoolRepository.callback be a list instead? I've made it a reference to
a single callback for performance reasons. I can change it if Kabir or Ales prefer.
4. Maybe we should rename ClassLoaderRepository to avoid confusion with
ClassPoolRepository class.
5.In (AOP)VFSClassLoaderDomain, instead of the initMapsDone and cleanupLoaderDone methods,
I could just override initMaps() and cleanupLoader() directly in AOPVFSClassLoaderDomain,
call the super method and add the "Done" stuff at the end. Kabir pointed out
that this may be not so simple as it looks because I need some local variables from those
methods.
I'm going to work on these points before porting the changes to trunk.
Notice that I'll have to check if the tests are still passing and do any adaptation
needed on the 742 branch... This can be done only after the classpool project is
estabilized and the vfs tests are passing ok. Those tests are the subject of another
thread.
http://www.jboss.org/index.html?module=bb&op=viewtopic&p=4265325#...
View the original post :
http://www.jboss.org/index.html?module=bb&op=viewtopic&p=4266376#...
Reply to the post :
http://www.jboss.org/index.html?module=bb&op=posting&mode=reply&a...