chia7712 commented on code in PR #15289:
URL: https://github.com/apache/kafka/pull/15289#discussion_r1511231627


##########
core/src/test/scala/unit/kafka/utils/TestUtils.scala:
##########
@@ -137,7 +137,9 @@ object TestUtils extends Logging {
     val parentFile = new File(parent)
     parentFile.mkdirs()
 
-    JTestUtils.tempDirectory(parentFile.toPath, null)
+    val result = JTestUtils.tempDirectory(parentFile.toPath, null)

Review Comment:
   >I feel it's semantically better for the function that creates parentFile to 
handle its deletion rather than delegating it to JTestUtils.tempDirectory since 
it might not be very obvious to callers of JTestUtils.tempDirectory otherwise.
   
   I agree that `JTestUtils.tempDirectory` is not obvious about deleting. 
However, it seems to me that is the problem. Callers can't ensure the duty of 
"cleanup", and so maybe the consistent behavior is more important here - if we 
decide to cleanup parent folder also, we should do same thing for java code.
   
   BTW, we can have a better naming if we assume `tempRelativeDir` needs to 
delete parent folder.



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to