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

Allow creating new Builders using BaseRecordBuilder #26

Merged
merged 1 commit into from
Apr 26, 2019

Conversation

bltavares
Copy link
Contributor

The current BaseRecordBuilder interface defines some abstract methods,
without declaring their visibility.

This means it is by default, they are package-private, and we can't
create sub-classes extending the behaviour.

One scenario where extanding from another package is ideal would be when
extra headers are required to be sent to Kafka. This commit changes the
modifier to be public so others could extend them.

The current `BaseRecordBuilder` interface defines some abstract methods,
without declaring their visibility.

This means it is by default, they are `package-private`, and we can't
create sub-classes extending the behaviour.

One scenario where extanding from another package is ideal would be when
extra headers are required to be sent to Kafka. This commit changes the
modifier to be public so others could extend them.
@AndrewJSchofield
Copy link
Contributor

This looks like a useful change. I'll take a look when I get some free time.

Copy link
Contributor

@AndrewJSchofield AndrewJSchofield 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 PR. I'm happy with the changes but you must accept the Contributor License Agreement to contribute to this repo. Just add the appropriate comment in the PR conversation. Thanks.

@bltavares
Copy link
Contributor Author

I accept the CLA

@AndrewJSchofield AndrewJSchofield merged commit 3a73dce into ibm-messaging:master Apr 26, 2019
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