MINOR: Improve IntegrationTestUtils documentation#5664
MINOR: Improve IntegrationTestUtils documentation#5664vahidhashemian merged 2 commits intoapache:trunkfrom
Conversation
vvcephei
left a comment
vvcephei
left a comment
There was a problem hiding this comment.
Thanks for the cleanup @dongjinleekr !
This looks good to me.
|
Is there anyone who can merge this PR into 2.1.0 release? |
|
@lindong28 Excuse me. Could this be included in 2.1.0? |
dongjinleekr
force-pushed
the
feature/integration-test-utils-javadoc
branch
from
59b506b to
1003515
Compare
…eters in methods. 3. Fix typo and method ordering.
dongjinleekr
force-pushed
the
feature/integration-test-utils-javadoc
branch
from
1003515 to
6c13b37
Compare
|
@vahidhashemian Excuse me. Could have a look? |
vahidhashemian
left a comment
vahidhashemian
left a comment
There was a problem hiding this comment.
Thanks for the PR. Just one question inline.
vvcephei
left a comment
vvcephei
left a comment
There was a problem hiding this comment.
Hi @dongjinleekr ,
Sorry for the silence... I think the only controversial portion is the new test util method. Otherwise, this PR has my approval (for what it's worth).
Maybe @guozhangwang can take a look?
-John
…p(String, Collection<KeyValue<K, V>>, Properties, Headers, Long)
|
@vvcephei @vahidhashemian Here it is. I removed the method. |
|
@dongjinleekr Kafka 2.1.0 was release 4 month ago. We can get it into 2.2.0 thought. Also, it's only test code, so we could, if we want back port it to get it into 2.1.1 -- however, @cmccabe is cutting 2.1.1-rc0 soon, so it might not make it -- also not worth blocking 2.1.1 release for this. Retest this please. |
|
@mjsax: agreed. |
vahidhashemian
left a comment
vahidhashemian
left a comment
There was a problem hiding this comment.
LGTM. Thanks for addressing the earlier comments.
* ak/trunk: MINOR: Update usage of deprecated API (apache#6146) KAFKA-4217: Add KStream.flatTransform (apache#5273) MINOR: Update Gradle to 5.1.1 (apache#6160) KAFKA-3522: Generalize Segments (apache#6170) Added quotes around the class path (apache#4469) KAFKA-7837: Ensure offline partitions are picked up as soon as possible when shrinking ISR (apache#6202) MINOR: In the MetadataResponse schema, ignorable should be a boolean KAFKA-7838: Log leader and follower end offsets when shrinking ISR (apache#6168) KAFKA-5692: Change PreferredReplicaLeaderElectionCommand to use Admin… (apache#3848) MINOR: clarify why suppress can sometimes drop tombstones (apache#6195) MINOR: Upgrade ducktape to 0.7.5 (apache#6197) MINOR: Improve IntegrationTestUtils documentation (apache#5664) MINOR: upgrade to jdk8 8u202 KAFKA-7693; Fix SequenceNumber overflow in producer (apache#5989) KAFKA-7692; Fix ProducerStateManager SequenceNumber overflow (apache#5990) MINOR: update copyright year in the NOTICE file. (apache#6196) KAFKA-7793: Improve the Trogdor command line. (apache#6133)
* 1. Add Javadoc to undocumented methods. 2. Add documentation on parameters in methods. 3. Fix typo and method ordering. * Remove IntegrationTestUtils#produceKeyValuesSynchronouslyWithTimestamp(String, Collection<KeyValue<K, V>>, Properties, Headers, Long)
5 participants
While I was writing an example project on testing Kafka Streams, I found that there are several problems with the documentation of
IntegrationTestUtils.Timeparameter inproduceKeyValuesSynchronouslymethods is missing. This parameter was added in commit de1b853, but documentation was omitted then.enableTransactionsinproduceKeyValuesSynchronouslyWithTimestamp(String, Collection, Properties, Headers headers, Long, boolean)intoenableTransactions: for consistency with the other overloads.produceKeyValuesSynchronouslyWithTimestamp,waitUntilMinRecordsReceived, etc.waitUntilMinRecordsReceivedCommitter Checklist (excluded from commit message)