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

Allow duplicate placeholder configs if desired #37

Merged

Conversation

sdwr98
Copy link

@sdwr98 sdwr98 commented Oct 5, 2015

Hi all,

I was very excited to see this project - we're facing the same architectural challenges that you are. One thing, though, is that in Spring MVC web applications, there is often a need to have PropertyPlaceholderConfigurers in both the root application context and the dispatcher servlet application context (because Bean Post Processors can't cross parent-child context boundaries).

This change allows the user to specify that they're ok if multiple placeholder configurers register, and will just use the config of the first one that makes it.

I'm open to feedback, but this helped us tremendously.

Thanks!
Scott

@buildhive
Copy link

Capgemini » archaius-spring-adapter #54 SUCCESS
This pull request looks good
(what's this?)

@andrewharmellaw
Copy link

Hi @sdwr98 . It's very cool that your itch is also our itch. And thanks for the contribution. I'll take a look at your PR. The cloudbees build looks good for starters...
Andrew

@andrewharmellaw
Copy link

One comment - could you add some tests? (E.g. check it works as expected, check it fails as expected, with the various ways you can config a Spring project). Just use the existing ones in src/test as examples (they're in Spock but thats pretty easy to read).

Then I think we're good to go.

@sdwr98
Copy link
Author

sdwr98 commented Oct 6, 2015

@andrewharmellaw Whoops - sorry about that. I didn't even see those hiding in the groovy folder. I've added some tests, let me know what you think.

@buildhive
Copy link

Capgemini » archaius-spring-adapter #55 SUCCESS
This pull request looks good
(what's this?)

@andrewharmellaw
Copy link

All looks good now. Apologies for taking so long to merge. Doing it now. Thanks again.

andrewharmellaw added a commit that referenced this pull request Nov 16, 2015
Allow duplicate placeholder configs if desired
@andrewharmellaw andrewharmellaw merged commit 5deb3c3 into Capgemini:master Nov 16, 2015
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.

None yet

3 participants