zisding commented on PR #514: URL: https://github.com/apache/tomcat/pull/514#issuecomment-1133097834
Hi Mark, @markt-asf Thanks for merging the PR. Really hope my contribution can make Tomcat more perfect, even if it is incremental. Based on the feedback, I further reviewed some source code and logging messages and found the following inaccurate expression of the relation between the logging statement and its corresponding action in the source code. Meanwhile, I also proposed some suggestions. Could you please help me figure out whether they are appropriate or not? I can make a PR later. 1. Logging statement: https://github.com/apache/tomcat/blob/b78d415754b8d9c3160f84c111a993533568e8df/java/org/apache/catalina/session/DataSourceStore.java#L537 Suggestion: Change `Removing` to `Removed` in the localstrings, `dataSourceStore.removing=Removing Session [{0}] at database [{1}]` Reason: as the logging statement is placed at the end of the function `remove()` indicting the function has been successfully executed. 2. Logging statement: https://github.com/apache/tomcat/blob/b78d415754b8d9c3160f84c111a993533568e8df/java/org/apache/catalina/valves/RequestFilterValve.java/#L383 Suggestion: Change `Denied` to `Denying` in the localstrings, `requestFilterValve.deny=Denied request for [{0}] based on property [{1}]` Reason: the logging statement is placed before the target function `denyRequest()` indicting the function will be executed. 3. Logging statement: https://github.com/apache/tomcat/blob/b78d415754b8d9c3160f84c111a993533568e8df/java/org/apache/catalina/realm/JAASRealm.java/#L569 Suggestion: Change `Adding` to `Added` in the localstrings, `jaasRealm.rolePrincipalAdd=Adding role Principal [{0}] to this user Principal''s roles`, Reason: the logging statement is placed after the target function `add()` indicting the status of `add` is completed. 4. Logging statement: https://github.com/apache/tomcat/blob/b78d415754b8d9c3160f84c111a993533568e8df/java/org/apache/catalina/ha/session/DeltaManager.java/#L1385 Suggestion: Change `Sent` to `Sending` in the localstrings, `deltaManager.createMessage.allSessionData=Manager [{0}] sent all session data.`, Reason: the logging statement is placed before the target function `send()` indicting the status of `send` will be executed or in progress. 5. Logging statement: https://github.com/apache/tomcat/blob/0f2e084d1583cfa4c00cec1958ec30113fb4f41e/java/org/apache/coyote/AbstractProtocol.java#L392 Suggestion: Change `Removed` to `Removing` in the localstrings, `abstractProtocol.waitingProcessor.remove=Removed processor [{0}] from waiting processors`, Reason: the logging statement is placed before the target function `remove()` indicting the status of `remove` will be executed. 6. Logging statement: https://github.com/apache/tomcat/blob/0f2e084d1583cfa4c00cec1958ec30113fb4f41e/java/org/apache/coyote/http2/Stream.java#L655 Suggestion: Change `sent ` to `sending` in the localstrings, `stream.reset.send=Connection [{0}], Stream [{1}], Reset sent due to [{2}]`, Reason: the logging statement is placed before the target function `sendReset()` indicting the status of `sendReset ` will be executed. 7. Logging statement: https://github.com/apache/tomcat/blob/0f2e084d1583cfa4c00cec1958ec30113fb4f41e/java/org/apache/coyote/http2/Stream.java/#L686 Suggestion: Change `has been recycled ` to `will be recycled ` in the localstrings, `stream.recycle=Connection [{0}], Stream [{1}] has been recycled`, Reason: the logging statement is placed before the target source code indicting the status of `recycle ` will be executed or in progress. Thanks for reviewing. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
