Skip to content

MINOR: Apply try-with-resource to KafkaStreamsTest#10668

Merged
mjsax merged 1 commit intoapache:trunkfrom
vitojeng:apply-try-with-resources
Jun 3, 2021
Merged

MINOR: Apply try-with-resource to KafkaStreamsTest#10668
mjsax merged 1 commit intoapache:trunkfrom
vitojeng:apply-try-with-resources

Conversation

@vitojeng
Copy link
Copy Markdown
Contributor

@vitojeng vitojeng commented May 11, 2021

In the PR #9821, @mjsax 's comment

We should use a try-with-resources clause to make sure close() is called.

Seems, other tests in this class have a similar issue. Would be good to fix all test accordingly (if you want you can also to a separate PR for it).

Let me address this before the KAFKA-5876(KIP-216) part 5.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@vitojeng vitojeng force-pushed the apply-try-with-resources branch from 46babe0 to d91a8e4 Compare May 11, 2021 07:33
@vitojeng
Copy link
Copy Markdown
Contributor Author

@mjsax , @ableegoldman
Please take a look. :)

Copy link
Copy Markdown
Member

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vitojeng thanks for this patch. one small comment below.

Comment thread streams/src/test/java/org/apache/kafka/streams/KafkaStreamsTest.java Outdated
@mjsax mjsax added the streams label May 11, 2021
Comment thread streams/src/test/java/org/apache/kafka/streams/KafkaStreamsTest.java Outdated
Comment thread streams/src/test/java/org/apache/kafka/streams/KafkaStreamsTest.java Outdated
@mjsax
Copy link
Copy Markdown
Member

mjsax commented Jun 2, 2021

@vitojeng -- Seems there is some conflicts. Can you rebase your PR so we can merge it?

@vitojeng vitojeng force-pushed the apply-try-with-resources branch from d91a8e4 to c53e84a Compare June 2, 2021 22:52
@vitojeng
Copy link
Copy Markdown
Contributor Author

vitojeng commented Jun 2, 2021

Thanks @mjsax !
Please take a look.🙂

@mjsax mjsax merged commit c2c08b4 into apache:trunk Jun 3, 2021
@mjsax
Copy link
Copy Markdown
Member

mjsax commented Jun 3, 2021

Thanks @vitojeng! Merged to trunk.

@vitojeng vitojeng deleted the apply-try-with-resources branch June 3, 2021 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants