Github user fjodorver commented on the pull request:
https://github.com/apache/tomcat/pull/21#issuecomment-110630412
2,4,5) I use ant validate for checkstyle validation. Is it enough?
3) It looks like we need to rewrite AuthConfigFactory anyway (for example,
possible memory leaks, huge method and so on). I personally prefer to introduce
small methods, which makes code reading much easier, because they work as
self-commented code. Also, it simplifies code testing. As a bonus, in simple
methods have shorter lines. For example, in current implementation need to
introduce an ugly final variable, instead of just getting the correct value and
make the first variable final. Second thing is guard clauses - usually I prefer
to make such checks in the beginning of the method. It's quite good to get rid
of necessary indentation and makes code lines shorter.
6. I've refined method order and some signatures (there are runtime
exceptions declared, which is not necessary). However, constants in
AuthConfigFactory are used for internal purposes, so I'd proposed security
management as separate patch.
---
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
