-
Notifications
You must be signed in to change notification settings - Fork 13
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
StrimziKafkaCluster
builder
#86
Conversation
Signed-off-by: see-quick <maros.orsak159@gmail.com>
Signed-off-by: see-quick <maros.orsak159@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the builder pattern is useful. But by removing the original constructors, you are breaking the compatibility. Is that wise / intended? Should we keep them in some way?
Hmmm, I was thinking about it, but adding more constructors would result in a lot of unnecessary code. Maybe a less aggressive approach would be to have both (but |
I think deprecating them would be fine. The question is if you can combine both the deprecated constructors and the builders to allow users to transition smoothly. |
Yeah, it would be okay I think. It would be a matter of extracting the same code into methods so we don't end up with repetitive code. I will update this PR ten :) |
Signed-off-by: see-quick <maros.orsak159@gmail.com>
Signed-off-by: see-quick <maros.orsak159@gmail.com>
src/main/java/io/strimzi/test/container/StrimziKafkaCluster.java
Outdated
Show resolved
Hide resolved
Signed-off-by: see-quick <maros.orsak159@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
This PR adds flexibility to
StrimziKafkaCluster
by adapting the Builder pattern. With such change we will easily extend such class adding more parameters. Solves #84.