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