Skip to content

Replace describeStream with listStream #164

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Jun 15, 2020
Merged

Replace describeStream with listStream #164

merged 9 commits into from
Jun 15, 2020

Conversation

mwesox
Copy link
Contributor

@mwesox mwesox commented Jun 10, 2020

@mwesox mwesox changed the title fix Replace describeStream with listStream Jun 10, 2020
artembilan
artembilan requested changes Jun 11, 2020
View reviewed changes
Copy link
Member

@artembilan artembilan 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 fix.

Would you mind to reconsider changes to the classes where you only fix a Copyright year?
We don't change that date until we really do some changes in the class.

Thanks for understanding!

@mwesox
Copy link
Contributor Author

mwesox commented Jun 12, 2020

Thanks for the fix.

Would you mind to reconsider changes to the classes where you only fix a Copyright year?
We don't change that date until we really do some changes in the class.

Thanks for understanding!

done!

artembilan
artembilan requested changes Jun 15, 2020
View reviewed changes
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

Too many not feature related code reformatting...
Please, consider in the future to concentrate just on the issue-related fixes.

Or you can add one commit for the real fix and another for code style reformatting to make it easier to review.

Please, address my two concerns for now and I'll merger this shortly.

Thanks

.../org/springframework/integration/aws/inbound/kinesis/KinesisMessageDrivenChannelAdapter.java Outdated

/**
* Sets the interval between 2 checkpoints. Only used when checkpointMode is periodic.
*
Copy link
Member

Choose a reason for hiding this comment

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

No blank lines in method JavaDoc, please: https://github.com/spring-projects/spring-framework/wiki/Code-Style#javadoc-formatting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

.../org/springframework/integration/aws/inbound/kinesis/KinesisMessageDrivenChannelAdapter.java Outdated

if (retryCount > this.describeStreamRetries) {
throw new IllegalStateException(
"Kinesis could not read shards from stream with name:[" + stream + "] ");
Copy link
Member

Choose a reason for hiding this comment

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

Need space between : and [.
Although do we need a colon at all...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@mwesox
- remove blanks from javadoc
dde7644 
- revert unnecassary formatting changes
@artembilan artembilan merged commit 10e96e2 into spring-projects:master Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@artembilan artembilan artembilan requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Replace describeStream with listStream

2 participants

@mwesox @artembilan