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

Usability improvements: New SuperStream builder #2541

Conversation

kurenchuksergey
Copy link
Contributor

builder provide a way to configure:

  • maxAge
  • maxLength
  • maxSegmentSize

The Issue: #2540

builder provide a way to configure:
- maxAge
- maxLength
- maxSegmentSize
Copy link
Contributor

@garyrussell garyrussell 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 contribution!

Please add some tests.

Fix style tests and add a new one for the super stream builder
Copy link
Contributor

@garyrussell garyrussell left a comment

Choose a reason for hiding this comment

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

Good work; thanks!

@@ -37,7 +38,6 @@
*
* @author Gary Russell
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add yourself to the author list.

/**
* Creates a builder for Super Stream.
*
* @param name stream name
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @param name stream name
* @param name stream name

Remove extra spaces, we don't align param descriptions by column.


/**
* Set the stream name.
*
Copy link
Contributor

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 javadocs (this one and others below).

* @author Sergei Kurenchuk
* @since 3.1
*/
public class SuperStreamConfigurationTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public class SuperStreamConfigurationTest {
public class SuperStreamConfigurationTests {

Our convention is ...Tests.

@garyrussell
Copy link
Contributor

Also, you mentioned x-initial-cluster-size in the issue.

Covered x-initial-cluster-size. Fixes after review
@garyrussell
Copy link
Contributor

Thanks! Merged as 3620f11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants