https://issues.apache.org/bugzilla/show_bug.cgi?id=54239

--- Comment #9 from Sheldon Shao <xs...@ebay.com> ---
(In reply to comment #3)
> Comment on attachment 29685 [details]
> ELInterpreterFactory
> 
> In principle this looks like a good idea.
> 
> I have a couple of concerns with the patch as currently written:
> 1. No documentation.
     ~~~~~~~~~~~~~~~~~ Added more comments in ELInterpreter &
ELInterpreterFactory
> 2. No test cases.
     ~~~~~~~~~~~~~~~~~ Provided a test case for ELInterpreterFactory

> 3. The use of enum for the default instance is rather odd.
     ~~~~~~~~~~~~~~~~~ Fixed

> 4. I dislike the use of system properties when they are not necessary. If
> the class name was handled as a servlet context initialization parameters
> then Tomcat already has the necessary plumbing for global, per host and per
> web application configuration.
     ~~~~~~~~~~~~~~~~~~ Supported passing the class name from context
initialization parameters. Still keep the feature from System Property. 
     In our case, System Property can be easily passed in from such kind of
Daemon-Watcher, For example: JSW.

> 5. Error messages need to use the standard i18n support.
     ~~~~~~~~~~~~~~~~~~ Fixed

-- 
You are receiving this mail because:
You are the assignee for the bug.

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

Reply via email to