Skip to content

MINOR: update javadoc on ReadOnlyWindowStore#2811

Closed
dguy wants to merge 2 commits intoapache:trunkfrom
dguy:minor-window-fetch-java-doc
Closed

MINOR: update javadoc on ReadOnlyWindowStore#2811
dguy wants to merge 2 commits intoapache:trunkfrom
dguy:minor-window-fetch-java-doc

Conversation

@dguy
Copy link
Copy Markdown
Contributor

@dguy dguy commented Apr 5, 2017

Highlight that the range in fetch is inclusive of both timeFrom and timeTo

@dguy
Copy link
Copy Markdown
Contributor Author

dguy commented Apr 5, 2017

@mjsax @enothereska @miguno

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 5, 2017

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

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 5, 2017

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

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 5, 2017

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

Copy link
Copy Markdown
Member

@mjsax mjsax left a comment

Choose a reason for hiding this comment

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

Thanks Damian!

@mjsax
Copy link
Copy Markdown
Member

mjsax commented Apr 5, 2017

@ijuma Can you cherry-pick this to 0.10.2, too?

Copy link
Copy Markdown
Member

@ijuma ijuma left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, LGTM with some minor changes. If you're OK with them, I'll make them during the merge.

* +--------------------------------
* </pre>
* And we called {@code store.fetch("A", 10, 20)} then the results will contain the first
* three windows from the table above, i.e., all those were start time >= 10 and <= 20.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor typo, were -> where? Also, 10 <= start time <= 20 might be slightly better notation.

* | A | 25 | 35 |
* +--------------------------------
* </pre>
* And we called {@code store.fetch("A", 10, 20)} then the results will contain the first
Copy link
Copy Markdown
Member

@ijuma ijuma Apr 6, 2017

Choose a reason for hiding this comment

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

called -> call?

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 6, 2017

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

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Apr 6, 2017

Merged to trunk and 0.10.2.

asfgit pushed a commit that referenced this pull request Apr 6, 2017
Highlight that the range in `fetch` is inclusive of both `timeFrom` and `timeTo`

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

Reviewers: Michael G. Noll <michael@confluent.io>, Matthias J. Sax <matthias@confluent.io>, Ismael Juma <ismael@juma.me.uk>

Closes #2811 from dguy/minor-window-fetch-java-doc

(cherry picked from commit b611cfa)
Signed-off-by: Ismael Juma <ismael@juma.me.uk>
@asfgit asfgit closed this in b611cfa Apr 6, 2017
@asfbot
Copy link
Copy Markdown

asfbot commented Apr 6, 2017

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

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 6, 2017

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

@dguy dguy deleted the minor-window-fetch-java-doc branch May 16, 2017 14:05
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.

5 participants