Skip to content

MINOR: Improve API docs of (flatT|t)ransform#6365

Merged
bbejeck merged 3 commits intoapache:trunkfrom
cadonna:api_docs_transform
Mar 7, 2019
Merged

MINOR: Improve API docs of (flatT|t)ransform#6365
bbejeck merged 3 commits intoapache:trunkfrom
cadonna:api_docs_transform

Conversation

@cadonna
Copy link
Copy Markdown
Member

@cadonna cadonna commented Mar 4, 2019

This commit is a follow-up of pull request #5273.

  • Adds and reformulates some parts
  • Removes parameters from links to methods in the javadocs

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

This commit is a leftover of pull request apache#5273.
@mjsax mjsax added the streams label Mar 4, 2019
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 for the follow up @cadonna! One clarification to my original comment.

* a schedule must be registered.
* The {@link Transformer} must return a {@link KeyValue} type in {@link Transformer#transform(Object, Object)
* transform()} and {@link org.apache.kafka.streams.processor.Punctuator#punctuate(long) punctuate()}.
* transform(K, V)} and {@link org.apache.kafka.streams.processor.Punctuator#punctuate(long) punctuate(long)}.
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.

We don't add parameters in any JavaDocs. We use () in purpose to "shorten" long parameter lists (if usr follows the link, she sees the full parameter list). Note that

{@link org.apache.kafka.streams.processor.Punctuator#punctuate(long)}

and

{@link org.apache.kafka.streams.processor.Punctuator#punctuate(long) punctuate(long)}

would be the same (both render as punctuate(long)), but we want to get simplified punctuate().

Similar below.

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.

I removed the parameters from the links in the parts that I modified. However, in the other parts there are also links with parameters, e.g., line 544. Should I also change those in this PR?

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.

I was just looking across the JavaDocs. Seems we are not very consistent in what we do... L544 is not the only example. I leave it up to you if you want to clean it up, or not. We also just merge this PR as-is. Might not be worth the time?

Copy link
Copy Markdown
Member Author

@cadonna cadonna Mar 5, 2019

Choose a reason for hiding this comment

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

I removed the parameters from all the links to methods in the javadocs for transform and flatTransform.

cadonna added 2 commits March 4, 2019 22:59
Additionally, reformulated the part about type-safety at compile-time.
- removes more parameters from links in javadocs
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.

LGTM.

Call for second review (any of @guozhangwang @bbejeck @vvcephei @ableegoldman)

Copy link
Copy Markdown
Member

@bbejeck bbejeck left a comment

Choose a reason for hiding this comment

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

LGMT

@bbejeck bbejeck changed the title Improve API docs of (flatT|t)ransform MINOR: Improve API docs of (flatT|t)ransform Mar 7, 2019
@bbejeck bbejeck merged commit 96c4e32 into apache:trunk Mar 7, 2019
@bbejeck
Copy link
Copy Markdown
Member

bbejeck commented Mar 7, 2019

Merged #6365 into trunk.

Thanks for the followup PR @cadonna!

bbejeck pushed a commit that referenced this pull request Mar 7, 2019
This commit is a follow-up of pull request #5273

Reviewers: Matthias J. Sax <mjsax@apache.org>, Bill Bejeck <bbejeck@gmail.com>
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
This commit is a follow-up of pull request apache#5273

Reviewers: Matthias J. Sax <mjsax@apache.org>, Bill Bejeck <bbejeck@gmail.com>
@cadonna cadonna deleted the api_docs_transform branch October 21, 2019 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants