Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLINK-7038] [docs] Correct misused term to KeyedStream #4229

Closed
wants to merge 1 commit into from

Conversation

rice668
Copy link

@rice668 rice668 commented Jun 30, 2017

Correct misused term to KeyedStream.

@tzulitai
Copy link
Contributor

@zhangminglei thanks for picking this up.

The last time I checked, there's much more misused "KeyedDataStream" terms, both in Javadocs and the documentation. Could you please include fixes for all of those with this PR?

@rice668
Copy link
Author

rice668 commented Jun 30, 2017

Okay. I really found many KeyedDataStream terms existing in flink project under master branch. But there is not a class called it. So, it is very strange. Also the same stuff refers to WindowedDataStream in flink. Could you check it again and makes sure I am correct or incorrect.

@tzulitai
Copy link
Contributor

tzulitai commented Jun 30, 2017

Yes. That's why the KeyedDataStream is a misused term. The actual class is called KeyedStream. That would also go for the same for WindowedDataStream, I think the class is actually called WindowedStream.

@rice668
Copy link
Author

rice668 commented Jun 30, 2017

Exactly, Could we create another jira or just do a hotfix for WindowedDataStream under FLINK-7038 ?

@rice668
Copy link
Author

rice668 commented Jun 30, 2017

Hi, @tzulitai I have updated the code. Please check whether it should add more ?

Copy link
Contributor

@tzulitai tzulitai left a comment

Choose a reason for hiding this comment

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

There seems to be 17 occurrences of the misused term. You've only corrected 13 of them.
A quick search in the IDE should be able to reveal all of them.

@@ -33,7 +33,7 @@
* <p>The state is accessed and modified by user functions, and checkpointed consistently
* by the system as part of the distributed snapshots.
*
* <p>The state is only accessible by functions applied on a KeyedDataStream. The key is
* <p>The state is only accessible by functions applied on a KeyedStream. The key is
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also change these to be actual Javadoc links?
i.e. {@link KeyedStream}.

Copy link
Author

Choose a reason for hiding this comment

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

I remember next time as I did not know before like this style. thanks. :)

@@ -29,7 +29,7 @@
* <p>The state is accessed and modified by user functions, and checkpointed consistently
* by the system as part of the distributed snapshots.
*
* <p>The state is only accessible by functions applied on a KeyedDataStream. The key is
* <p>The state is only accessible by functions applied on a KeyedStream. The key is
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also change these to be actual Javadoc links?
i.e. {@link KeyedStream}.

Copy link
Author

Choose a reason for hiding this comment

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

I probably not set this to @link , instead, I would set this to @code, because if I use link, it can not be recognized by compiler. I have to add the pom.xml to the module. And I think it is not suitable. What do you think ?

@@ -28,7 +28,7 @@
* <p>The state is accessed and modified by user functions, and checkpointed consistently
* by the system as part of the distributed snapshots.
*
* <p>The state is only accessible by functions applied on a KeyedDataStream. The key is
* <p>The state is only accessible by functions applied on a KeyedStream. The key is
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also change these to be actual Javadoc links?
i.e. {@link KeyedStream}.

@@ -23,7 +23,7 @@
/**
* Interface that different types of partitioned state must implement.
*
* <p>The state is only accessible by functions applied on a KeyedDataStream. The key is
* <p>The state is only accessible by functions applied on a KeyedStream. The key is
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also change these to be actual Javadoc links?
i.e. {@link KeyedStream}.

@rice668
Copy link
Author

rice668 commented Jul 1, 2017

Yep. I will update code again soon enough.

@tzulitai
Copy link
Contributor

tzulitai commented Jul 4, 2017

LGTM, merging ..

tzulitai added a commit to tzulitai/flink that referenced this pull request Jul 5, 2017
tzulitai added a commit to tzulitai/flink that referenced this pull request Jul 5, 2017
tzulitai added a commit to tzulitai/flink that referenced this pull request Jul 5, 2017
tzulitai added a commit to tzulitai/flink that referenced this pull request Jul 5, 2017
@asfgit asfgit closed this in 5c6d797 Jul 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants