Skip to content

[FLINK-2818] [runtime] Corrected javadocs of *ReduceDriver classes - #1224

Closed
ggevay wants to merge 1 commit into
apache:masterfrom
ggevay:reduce-javadocs
Closed

[FLINK-2818] [runtime] Corrected javadocs of *ReduceDriver classes#1224
ggevay wants to merge 1 commit into
apache:masterfrom
ggevay:reduce-javadocs

Conversation

@ggevay

@ggevay ggevay commented Oct 4, 2015

Copy link
Copy Markdown
Contributor
  • "The iterator is handed to the reduce() method of the ReduceFunction."
    In the cases of ReduceDriver and AllReduceDriver, it is not the iterator which is handed to reduce(), but the elements pairwise
  • "The iterator returns all records grouped by their key"
    I guess this is not the case with AllReduceDriver and AllGroupReduceDriver. But someone who knows the code please confirm this!
  • AllGroupReduceDriver refers to "RichGroupFunction"
    There is no such thing. I guess this should be RichGroupReduceFunction.
  • GroupCombineChainedDriver refers to "GroupCombineDriver"
    There is no such thing. I guess this should be GroupReduceCombineDriver? (please confirm)
  • *Driver instead of *Task
  • In the combineGroup methods in DataSet, SortedGrouping, and UnsortedGrouping, the text of the NullPointerException is wrong.
  • The @return annotation of the first() method refers to "ReduceGroupOperator" instead of "GroupReduceOperator".
  • The javadoc of combineGroup() refers to CombineFunction instead of GroupCombineFunction
  • minor typos
  • CombineFunction is not used anywhere (except in comments), but I didn't do anything about this, because I'm not sure whether it should be removed, or perhaps there are plans for using it in the future.

@StephanEwen

Copy link
Copy Markdown
Contributor

Looks good, +1 to merge

BTW: I think its not necessary to {@link} the return types - they are linked anyways in the JavaDoc.

@mxm

mxm commented Oct 6, 2015

Copy link
Copy Markdown
Contributor

+1 to merge.

@chiwanpark

Copy link
Copy Markdown
Member

I'll merge this with addressing @StephanEwen's comment.

@asfgit asfgit closed this in 48f614c Oct 9, 2015
cfmcgrady pushed a commit to cfmcgrady/flink that referenced this pull request Oct 23, 2015
lofifnc pushed a commit to lofifnc/flink that referenced this pull request Oct 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants