zisding commented on PR #714: URL: https://github.com/apache/jmeter/pull/714#issuecomment-1146292194
Hi @pmouawad @FSchumacher , Thanks for approving the comments. Really hope my contribution can make Jmeter more perfect, though the contribution 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/jmeter/blob/659c1ff5eaea941eb7ad0638b58e904dcc06d961/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/DefaultSamplerCreator.java#L512 **Suggestion:** Change `setting port` to `finished setting port` **Reason:** The logging statement is placed after the function setPort() indicting the function has been successfully executed. 1. **Logging statement:** https://github.com/apache/jmeter/blob/659c1ff5eaea941eb7ad0638b58e904dcc06d961/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java#L988 **Suggestion:** Change `superseded by` to `will be superseded by` **Reason:** The logging statement is placed before the target function `setCacheManagerProperty()` indicting the function will be executed. 1. **Logging statement:** https://github.com/apache/jmeter/blob/659c1ff5eaea941eb7ad0638b58e904dcc06d961/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java#L1006 **Suggestion:** Change `superseded by` to `will be superseded by` **Reason:** The logging statement is placed before the target function `setProperty()` indicting the function will be executed. 1. **Logging statement:** https://github.com/apache/jmeter/blob/659c1ff5eaea941eb7ad0638b58e904dcc06d961/src/jorphan/src/main/java/org/apache/jorphan/collections/Data.java#L144 **Suggestion:** Move the logging statatement one line above, before the method ` setCurrentPos(index);` **Reason:** The logging statement is placed before the function `setCurrentPos()` indicting the function will be executed. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
