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

PIP-29: One package for both pulsar client and pulsar admin #3662

Merged
merged 1 commit into from
Feb 22, 2019

Conversation

sijie
Copy link
Member

@sijie sijie commented Feb 22, 2019

Motivation

Currently we are shipping pulsar-client and pulsar-client-admin separately. Both pulsar-client and pulsar-client-admin are shaded packages. But they shaded the dependencies independently.

It is quite common to see applications using both pulsar-client and pulsar-client-admin. These applications will have redundant shaded classes existed in both pulsar-client and pulsar-client-admin. Sometime it also causes troubles when we introduced new dependencies but forget to update shading rules.

Modifications

This proposal is proposing introduced a new module called pulsar-client-all. It will include both pulsar-client and pulsar-client-admin and shade the dependencies only one time. This would reduce the size of dependencies for applications using both pulsar-client and pulsar-client-admin.

*Motivation*

Currently we are shipping pulsar-client and pulsar-client-admin separately. Both pulsar-client and pulsar-client-admin are shaded packages. But they shaded the dependencies independently.

It is quite common to see applications using both pulsar-client and pulsar-client-admin. These applications will have redundant shaded classes existed in both pulsar-client and pulsar-client-admin. Sometime it also causes troubles when we introduced new dependencies but forget to update shading rules.

*Modifications*

This proposal is proposing introduced a new module called pulsar-client-all. It will include both pulsar-client and pulsar-client-admin and shade the dependencies only one time. This would reduce the size of dependencies for applications using both pulsar-client and pulsar-client-admin.
@sijie sijie added area/client type/feature The PR added a new feature or issue requested a new feature labels Feb 22, 2019
@sijie sijie added this to the 2.4.0 milestone Feb 22, 2019
@sijie sijie self-assigned this Feb 22, 2019
@sijie
Copy link
Member Author

sijie commented Feb 22, 2019

run java8 tests

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.

LGTM, though shouldn’t that be PIP-29 instead of BP-29? ;)

@sijie sijie changed the title BP-29: One package for both pulsar client and pulsar admin PIP-29: One package for both pulsar client and pulsar admin Feb 22, 2019
@sijie
Copy link
Member Author

sijie commented Feb 22, 2019

though shouldn’t that be PIP-29 instead of BP-29? ;)

You are right. Fixed :)

@merlimat
Copy link
Contributor

Can you also fix the commit log message, because that will still show up as bp on git

@sijie
Copy link
Member Author

sijie commented Feb 22, 2019

Can you also fix the commit log message, because that will still show up as bp on git

we are doing squash-and-merge. the commit log message is determined at merging time. we can fix it when merging it.

@merlimat merlimat merged commit 4376c65 into apache:master Feb 22, 2019
@merlimat
Copy link
Contributor

True.. it's just that by default it still uses the commit log message and people (me included) forget to update at the squash time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/client type/feature The PR added a new feature or issue requested a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants