kevoree / kevoree-library

Kevoree base library
GNU Lesser General Public License v3.0
2 stars 0 forks source link

injectDictionary on AddInstance #3

Closed edaubert closed 10 years ago

edaubert commented 10 years ago

Currently Planning generate AddInstance, updateDictionary commands when we add an instance with non default value for the dictionary.

In the AddInstance command, there is a call to injectDictionary which inject default value for the dictionary.

However, if we use setter to this parameter in order to detect when the value change, we get a change of the value because the AddInstance have already set them with the default value and the updateDictionary define the personalized values.

How can I know that this changes doesn't matter ? Or is it really necessary to injectDictionary on AddInstance ? Or is it really necessary to inject default value on AddInstance instead of good values ?

dukeboard commented 10 years ago

Hello, Normally the inject dictionary don't inject values when it is availables as oerriden values (not only defaults). IIn other word , it inject only if only default value are available

edaubert commented 10 years ago

Well, I have a test where the default value are loaded and then the one I set in my kevscript is set...

dukeboard commented 10 years ago

I'm perhaps a bug in bootstraper then / i check immedately / I have already fixed the Hook problem :-)

edaubert commented 10 years ago

Discussing with Maxime, We were thinking that instead of doing that on the AddInstance, it could be done on start for the defaultValue.

In that case, if the personalized values are already set, then the default values do not need to be set

dukeboard commented 10 years ago

No i think it really just a if to add in bootstrap because you will have troubles with setter othervise , it change nothing to call from start or add , it is the same method called ...

dukeboard commented 10 years ago

Should be fixed now in trunk , can you test ?

edaubert commented 10 years ago

Seems to be fixed with commit https://github.com/dukeboard/kevoree/commit/05586a2fa3e2e953ce59d724dc199bb40cd55d0e

dukeboard commented 10 years ago

Yes normally tested, thanks for detecting the bug, should be fine now :-) . Perhaps one additional task will be to add some tests in the library to ensure the non regression, but it is not hurry