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

Minimize the usage of guava classes in interfaces, return types, arguments #11501

Merged
merged 1 commit into from Jun 4, 2015

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Jun 4, 2015

Using these classes in interfaces forces downstream users to use shaded APIs.
We should minimize the usage of shaded APIs and keep them internal.

…ments

Using these classes in interfaces forces downstream users to use shaded APIs.
We should minimize the usage of shaded APIs and keep them internal.
@rmuir
Copy link
Contributor

rmuir commented Jun 4, 2015

+1

In addition to guava being certifiably useless, if you mix in shading and bake this stuff into apis, its about as wrong as it gets. Thanks for fixing this!

@rjernst
Copy link
Member

rjernst commented Jun 4, 2015

LGTM too

/**
* Combines multiple iterators into a single iterator.
*/
public static <T> Iterator<T> concat(Iterator<? extends T>... iterators) {
Copy link
Member

Choose a reason for hiding this comment

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

Why the wrappers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

long story - related to plugins...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this method is used in plugins a lot so I just wrap it here

Copy link
Member

Choose a reason for hiding this comment

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

Because you don't want them to use the shaded Guava because its an implementation detail?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah it's related to #11488

@s1monw s1monw merged commit 6a90c64 into elastic:master Jun 4, 2015
@s1monw s1monw removed the review label Jun 4, 2015
@s1monw s1monw deleted the no_shaded_api branch June 4, 2015 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants