On 24/09/2014 10:16, Konstantin Kolinko wrote: > 2014-09-24 12:29 GMT+04:00 Mark Thomas <ma...@apache.org>: >> On 24/09/2014 08:59, kkoli...@apache.org wrote: >>> Author: kkolinko >>> Date: Wed Sep 24 07:59:57 2014 >>> New Revision: 1627247 >>> >>> URL: http://svn.apache.org/r1627247 >>> Log: >>> Revert r1623471 + r1623804. >>> This was the fix for >>> https://issues.apache.org/bugzilla/show_bug.cgi?id=56401 >>> Log server information when Tomcat starts. >>> >>> Reason: It is wrong to log this information in constructor of Catalina >>> class, as I noted in Bugzilla. >>> There have to be a better solution. I think it is better to revert for now >>> to avoid delaying tagging of Tomcat 8.0.13. >> >> This is absolutely not the way an objection to a commit should be handled. >> >> If you object to a commit then you need to veto that commit, providing >> valid technical reasons for doing so. >> >> In most cases a discussion will then follow on the dev list. >> >> If the veto is considered valid by the community then the original >> committer is expected to revert it. >> >> If the original committer doesn't revert it after a suitable period then >> someone else can revert it but that should be the exception rather than >> the rule and in all cases it should be preceded by an e-mail to the dev >> list *before* the commit is reverted. > > > 1. There is both yours and mine code reverted here.
You can do what you like to your commits (subject to review by the other committers). You are expected to treat other people's commits with respect. > 2. The concern was raised 15 days ago, > http://markmail.org/message/cy2z3drad7jlru37 That is a comment, not a veto and Bugzilla is not the best place for discussions. "I think it should not perform logging on shutdown." Is a comment on something that needs to be improved. It is not a veto, nor is it notice that you intend to revert it. I do agree that that is a valid concern that needs to be addressed although I don't think it justifies reverting someone else's commit without any notice. > 3. This is a rather visible feature. If you intend to tag 8.0.13 in > the next 0-12 hours, I do not have enough time to test it or to > improve it by myself. Visible yes but the chances of it causing a regression are very, very small and nothing stops us changing how this is implemented in a later release. > Possible ways to go: > either > a) Change the place where logInfo() method is called from. > or > b) Move implementation to a separate component (Listener, e.g. > TomcatVersionDisplayListener or better name) and explicitly configure > it in default server.xml. My main concern is making sure that this appears first before any other log messages. A listener just to generate this seems like over kill on one hand. On the other, it is nice and modular and easy for folks to remove if they don't want it. Regarding one log message with newlines vs. several. I think several is better as log messages with new lines tend to cause problems for log parsers (stack traces being the typical exception that parsers can handle). This should be a simple thing to implement so I'll see if I can get something done before I tag 8.0.13. (That will also give folks a chance to look at the CredentialHandler patch although I suspect that will take longer to discuss and will be in a later version.) Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
