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

Move ConnectionStrategy interface into the base package #75

Merged
merged 2 commits into from Feb 25, 2016

Conversation

Projects
None yet
2 participants
@dfish3r
Copy link
Member

dfish3r commented Feb 25, 2016

No description provided.

Move ConnectionStrategy to base package.
ConnectionStrategy is now a property on ConnectionConfig.
@serac

This comment has been minimized.

Copy link
Member

serac commented on f499c1b Jun 10, 2015

I think moving ConnectionStrategy out of provider makes sense since it's not a provider-specific concern. I imagine most consumers will never user anything other than the default.

This comment has been minimized.

Copy link
Member

dfish3r replied Jun 10, 2015

I don't think the ConnectionStrategy interface received a lot of review since it was buried in the provider package. Do you think the interface is robust enough as is?

This comment has been minimized.

Copy link
Member

serac replied Jun 10, 2015

Seems like it has what it needs. I'd recommend growing as future requirements arise so we don't over-engineer. The only suggestion I might have is to move the fields to a separate ConnectionStrategies enum. No sense in polluting the interface with default implementations.

This comment has been minimized.

Copy link
Member

dfish3r replied Jun 10, 2015

That was done to keep some compatibility with the old class:
https://vt-middleware.googlecode.com/svn/ldaptive/core/tags/ldaptive-1.0.3/src/main/java/org/ldaptive/provider/ConnectionStrategy.java

But it seems like now is the time to fix it.

dfish3r added a commit that referenced this pull request Feb 25, 2016

Merge pull request #75 from vt-middleware/conn-strategy
Move ConnectionStrategy interface into the base package

@dfish3r dfish3r merged commit beb3164 into master Feb 25, 2016

@dfish3r dfish3r deleted the conn-strategy branch Feb 25, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment