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

Fix ConfigurationBuilder addClassLoaders #288

Merged

Conversation

dariocutillas
Copy link
Contributor

The addClassLoaders(ClassLoader ... classloaders) was trying to concatenate ClassLoader.class to the stream of classloaders, and the toArray() method would fail given that ClassLoader.class is not of type ClassLoader.

Notice that I could not understand why there is interest in adding the ClassLoader.class so my solution was to remove it.

If what we are seeking is that the class loader of the ClassLoader is to be added, then probably we are missing a ClassLoader.class.getClassLoader(). But doing so would be inconsistent as the following would not produce same output:

ConfigurationBuilder configurationBuilder = new ConfigurationBuilder().addClassLoader( foo ).addClassLoader( bar );
ConfigurationBuilder configurationBuilder = new ConfigurationBuilder().addClassLoader( foo, bar );

In the second case, the ClassLoader.class would be ignored, because the ClassLoader[] classLoaders field of the ConfigurationBuilder is null and then classLoaders would simply match the passed { foo, bar } array.

@ronmamo ronmamo changed the base branch from master to release/0913 April 26, 2020 14:18
@ronmamo ronmamo merged commit 81eb4a3 into ronmamo:release/0913 Apr 26, 2020
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