Skip to content

MINOR: update KTable JavaDoc#2438

Closed
mjsax wants to merge 3 commits intoapache:trunkfrom
mjsax:ktableJavaDocs
Closed

MINOR: update KTable JavaDoc#2438
mjsax wants to merge 3 commits intoapache:trunkfrom
mjsax:ktableJavaDocs

Conversation

@mjsax
Copy link
Copy Markdown
Member

@mjsax mjsax commented Jan 26, 2017

No description provided.

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Jan 26, 2017

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 26, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/1224/
Test FAILed (JDK 8 and Scala 2.12).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 26, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/1226/
Test PASSed (JDK 8 and Scala 2.11).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 26, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/1224/
Test PASSed (JDK 7 and Scala 2.10).

Copy link
Copy Markdown
Contributor

@dguy dguy left a comment

Choose a reason for hiding this comment

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

Some typos/grammar changes

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should actually remove the reference to KTable as we don't currently support it

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove "or multiple"
Unless there is something i'm missing a KTable is created from exactly one topic unless it is the result of an aggregation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Print the updated...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Was intended as is -- records are updates, so its an "update record" (or update-record). "updated" makes sense, too, though. Not sure what the better wording is...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see what you mean. I just think "updated records" sounds more natural. But i'm not overly concerned

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove out

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

updated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

as above

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if -> of ?
also add a , before and

Copy link
Copy Markdown
Member Author

@mjsax mjsax Jan 26, 2017

Choose a reason for hiding this comment

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

Why a comma?
To my knowledge, for two things that are listed there is no comma: "A and B"
Only for three or more, there are command: "A, B, C, and D"

Correct my if I am wrong.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

an -> a

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

as above

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

as above

@dguy
Copy link
Copy Markdown
Contributor

dguy commented Jan 26, 2017 via email

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Jan 27, 2017

Addressed @dguy comments. (Not the part about "update records" -- but we can discuss if we want to reword).

Also added KGroupedTable and GlobalKTable.

Some minor refactoring in KStream and KGroupedStream (always discovering small issues along the way...)

Call for review @eno @guozhangwang

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 27, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/1282/
Test PASSed (JDK 8 and Scala 2.11).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 27, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/1280/
Test PASSed (JDK 8 and Scala 2.12).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 27, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/1280/
Test PASSed (JDK 7 and Scala 2.10).

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Jan 27, 2017

Rebased to resolve conflict.

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 27, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/1284/
Test PASSed (JDK 8 and Scala 2.12).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 27, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/1286/
Test PASSed (JDK 8 and Scala 2.11).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 27, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/1284/
Test PASSed (JDK 7 and Scala 2.10).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 27, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/1287/
Test FAILed (JDK 7 and Scala 2.10).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 27, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/1289/
Test PASSed (JDK 8 and Scala 2.11).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 27, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/1287/
Test PASSed (JDK 8 and Scala 2.12).

asfgit pushed a commit that referenced this pull request Jan 27, 2017
Author: Matthias J. Sax <matthias@confluent.io>

Reviewers: Damian Guy <damian.guy@gmail.com>

Closes #2438 from mjsax/ktableJavaDocs

(cherry picked from commit 1bd11b2)
Signed-off-by: Guozhang Wang <wangguoz@gmail.com>
@guozhangwang
Copy link
Copy Markdown
Contributor

Merged to trunk and 0.10.2.

@asfgit asfgit closed this in 1bd11b2 Jan 27, 2017
@mjsax mjsax deleted the ktableJavaDocs branch January 28, 2017 01:24
soenkeliebau pushed a commit to soenkeliebau/kafka that referenced this pull request Feb 7, 2017
Author: Matthias J. Sax <matthias@confluent.io>

Reviewers: Damian Guy <damian.guy@gmail.com>

Closes apache#2438 from mjsax/ktableJavaDocs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants