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

unboundid.TestContextSourceFactoryBean should initialize the Server using the provided principal and password #578

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Interessierter
Copy link

@Interessierter Interessierter commented Apr 1, 2021

had to make some minor API adjustments but kept the old API for backward compatibility

…d test server

- had to make some minor API adjustments but kept the old API for backward compatibility
@rwinch rwinch changed the title #576 allow different credentials for UnboundId embedded test server Allow different credentials for UnboundId Embedded test server Apr 14, 2021
@rwinch rwinch changed the title Allow different credentials for UnboundId Embedded test server unboundid.TestContextSourceFactoryBean should initialize the Server using the provided principal and password Apr 14, 2021
Copy link
Member

@rwinch rwinch left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

I think we should restructure this a bit due to the number of parameters in the static methods. I'd prefer to EmbeddedLdapServer turned into a builder style API. The static methods would consume the builder. Rather than having the TestContextSourceFatoryBean consume the static methods it would use the builder directly. This will ensure if we need to add more options we don't have to expand the static methods to have even more arguments.

@pivotal-cla
Copy link

@Interessierter Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@jzheaux
Copy link
Contributor

jzheaux commented Jan 19, 2022

Hi, @Interessierter, thanks for the contribution. Are you able to make the requested changes?

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.

4 participants