On 06/12/2018 13:19, Romain Manni-Bucau wrote:
> Hello Rémy,
> 
> 
> Le jeu. 6 déc. 2018 à 14:13, Rémy Maucherat <r...@apache.org> a écrit :
> 
>> On Thu, Dec 6, 2018 at 11:51 AM Romain Manni-Bucau <rmannibu...@gmail.com>
>> wrote:
>>
>>> Hi guys,
>>>
>>> can you make ContextConfig.JavaClassCacheEntry public please? Idea is to
>> be
>>> able to override ContextConfig and potentially customize
>> processAnnotations
>>> methods. Currently it is a pain and it is preventing to upgrade
>> meecrowave
>>> and likely tomee to java11+jlink support
>>>
>>
>> Ok, so I had a look at the meecrowave main class after doing my embedded
>> refactoring and I was a bit horrified by the amount of hacks :) Not that
>> there were too many options to do the things that were done (some I didn't
>> think were possible), and luckily my changes would have been a very good
>> help there.
>>
>> So here, it looks like a bit scary as well, since JavaClassCacheEntry is
>> package private (not ok usually), but only instantiated from a private
>> method (what would you do about that ?). So I'd rather not do that, or not
>> just that, since it would be good to reduce the amount of hacks.
>>
>> For starters, I don't like "private" in that kind of class (same with
>> "final"), I prefer "protected" usually. Shouldn't JavaClassCacheEntry be a
>> non static protected class ?

No objections to making it protected if that helps but I don't see why
you'd want to make it non-static.

>> About webConfig, I'm not sure. A good way to do it would be to add calls to
>> a few intermediate empty methods in appropriate locations, rather than a
>> real refactoring.
> 
> not empty cause here the idea is to reuse an existing scanning and not let
> tomcat scan classes but yes a doScanMagic() would work
> the part which would be great to reuse is the mapping between bytecode (the
> annotation entry) and the tomcat model (FilterDef etc)

Why don't you suggest a patch? As a rough guideline, please aim for the
minimally invasive patch the gives you what you need.

Generally adding to the existing API is OK but changing it is not. The
aim is to minimise the chance of breakage for anyone currently using or
extending that class directly.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to