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

remove pulsar-client-schema #3184

Merged
merged 11 commits into from
Dec 14, 2018

Conversation

jerrypeng
Copy link
Contributor

@jerrypeng jerrypeng commented Dec 13, 2018

Motivation

Remove pulsar-client-schema module since its not really needed. Merge the module with pulsar-client-original. Reduce project complexity.

This change will also greatly reduce the size of some of the other packages e.g. datagenerator source NAR package from 35MB -> 7 MB

Please note I create a new module called pulsar-client-api to hold API interfaces/classes for pulsar. This is needed for schema related interfaces/classes since many other modules depend on them and it doesn't make since to pull in pulsar-client-original just for the schema API. After discussing with @merlimat, he agrees and recommends to move all API related interfaces/classes into a separate module, since it make it light weight and flexible for other to depend on as well as it works well with javadoc of which to current setup does not work because of shading. In this PR, I only moved a subset of the schema API Interfaces into the pulsar-client-api module as a start.

This PR is read for review

Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

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

Looks good!

@jerrypeng jerrypeng changed the title remove pulsar-client-schema (WIP) remove pulsar-client-schema Dec 14, 2018
@jerrypeng jerrypeng self-assigned this Dec 14, 2018
@jerrypeng jerrypeng added this to the 2.3.0 milestone Dec 14, 2018
@jerrypeng
Copy link
Contributor Author

This PR is ready for review

Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

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

👍

@jerrypeng
Copy link
Contributor Author

rerun integration tests

@merlimat merlimat merged commit 5dd4b6e into apache:master Dec 14, 2018
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.

3 participants