Thanks for taking a look Paul. Comment inline:

http://codereview.appspot.com/2000043/diff/13001/5003
File
java/common/src/main/java/org/apache/shindig/config/JsonContainerConfig.java
(left):

http://codereview.appspot.com/2000043/diff/13001/5003#oldcode378
java/common/src/main/java/org/apache/shindig/config/JsonContainerConfig.java:378:
// Now that all containers are loaded, we go back through them and merge
On 2010/08/25 06:46:26, Paul Lindner wrote:
This is the code that merges the default params, we should probably
split this
into it's own routine (mergeContainers?) and call it after we modify
the
container with values from the environment..


I don't have enough idea / confidence about refactoring this class yet.
I took a brief look at the mergeParents and mergeObjects method and it
seems we resolve all the way to the default container, and somehow merge
fields into the current container (i guess every1 knew that duhh)
Then i tried moving out the SERVER_HOST and SERVER_PORT code before this
merge business and my test seems to still pass. I guess whats happening
is that SERVER_HOST and SERVER_PORT fields of default container are
getting merged into all the containers.

Are you okay with this for now ? Please let me know if i'm missing out
something obvious.

http://codereview.appspot.com/2000043/

Reply via email to