Skip to content

[FLINK-6695] Activate strict checkstyle in flink-contrib - #4004

Closed
zentol wants to merge 5 commits into
apache:masterfrom
zentol:6695csctrb
Closed

[FLINK-6695] Activate strict checkstyle in flink-contrib#4004
zentol wants to merge 5 commits into
apache:masterfrom
zentol:6695csctrb

Conversation

@zentol

@zentol zentol commented May 28, 2017

Copy link
Copy Markdown
Contributor

This PR activates the strict checkstyle for most modules in flink-contrib; flink-tweet-inputformat is not included due to proposal to remove it in FLINK-6710.

@zentol zentol changed the title [FLINK-6695 Activate strict checkstyle in flink-contrib [FLINK-6695] Activate strict checkstyle in flink-contrib May 28, 2017

@greghogan greghogan left a comment

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.

+1 with minor comments


/**
* Verifies that incoming numbers are either even or odd, controlled by the constructor argument. Emitted tuples are
* enriched with a new string field containing either "even" or "odd", based on the numbers parity.

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.

numbers parity -> numbers' parity

import org.apache.storm.topology.TopologyBuilder;

/**
* A simple topology that splits a number stream based the numbers parity, and verifies the result.

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.

numbers -> numbers'

import java.util.Random;

/**
* A Spout implementation that broadcast random numbers across a specified number of output streams, until a specified

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.

"broadcast" -> "broadcasts"

"was" -> "is"?

import java.util.Map;

/**
* A Bolt implementation that verifies meta data emitted by a {@link MetaDataSpout}.

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 think that this should be "metadata" but see that the class is camelcased MetaData.

@zentol

zentol commented May 28, 2017

Copy link
Copy Markdown
Contributor Author

@greghogan fixed.


/**
* A simple topology that splits a number stream based the numbers parity, and verifies the result.
* A simple topology that splits a numbers stream based the numbers parity, and verifies the result.

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.

What if we simplified this to "splits a number stream based on the parity"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sounds better

/**
* A Spout implementation that broadcast random numbers across a specified number of output streams, until a specified
* count was reached.
* A Spout implementation that broadcast randoms numbers across a specified number of output streams, until a specified

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.

"broadcasts random"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

wow. I really added the s to the wrong word...

/**
* Verifies that incoming numbers are either even or odd, controlled by the constructor argument. Emitted tuples are
* enriched with a new string field containing either "even" or "odd", based on the numbers parity.
* enriched with a new string field containing either "even" or "odd", based on the numbers' parity.

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 think I got this wrong, too. Should probably be number's.

@zentol

zentol commented May 29, 2017

Copy link
Copy Markdown
Contributor Author

@greghogan fixed.


/**
* A simple topology that splits a numbers stream based the numbers parity, and verifies the result.
* A simple topology that splits a numbers stream based their parity, and verifies the result.

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.

This would be a "number stream" or a "stream of numbers"? And I think "based on ...".

@zentol

zentol commented May 31, 2017

Copy link
Copy Markdown
Contributor Author

@greghogan did another attempt at fixing the issues.

@greghogan

Copy link
Copy Markdown
Contributor

+1!

@zentol

zentol commented May 31, 2017

Copy link
Copy Markdown
Contributor Author

YES! :D merging this.

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.

3 participants