Github user ChristopherSchultz commented on the issue:

    https://github.com/apache/tomcat/pull/56
  
    A few comments:
    
    1. I think the patch should include some TRACE-level logging, especially 
when catching the NumberFormatException.
    2. If there is an unrecognized option string, it should at least result in 
a WARN message emitted to the log. I might even lobby for an exception to be 
thrown.
    3. There are options that seem to be in conflict with each other, if only 
in terminology: e.g. "async" and "sync" can be specified together, which would 
read like a bug in the config.
    4. Splitting on a comma should include optional whitespace e.g. "\\s,*\\s*" 
as the pattern.
    5. The patch needs to include documentation for the string-based 
configuration options (webapps/docs/cluster-howto.html).
    6. The javadoc in the patch needs to include the list of acceptable string 
option-names.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

Reply via email to