← Back to team overview

zorba-coders team mailing list archive

Re: [Merge] lp:~zorba-coders/zorba/feature-reference_module into lp:zorba

 

> Hi Matthias,
> 
> It looks good! I have the following comments:
> 
> - 104 : JSON reference -> did you mean JSON array?
fixed

> 
> - Why return an error if ref:dereference is called with an unrecognized URI,
> while returning the empty sequence if it is recognized but unbound? Can we
> unify and always return the empty sequence? Or is it part of the API that
> references have a certain format? I thought of them as blackbox URIs.
Good point. Unfortunately, I will not be able to fix it easily because the code
is shared with the old node-reference module. Changing it would be a backwards
incompatible change to the old module. Do you think it's worth it to special
case the old and new module in the code? If so, I can do it.

> 
> - The error code text only mentions objects not in a collection, but not
> arrays.
fixed

> 
> - With the changes pending in xml-in-json-indices, the code could become
> considerably simpler (less, perhaps even no more casts to XmlNode or
> JSONItem).
I know. Whatever gets merged first wins. ;-)

-- 
https://code.launchpad.net/~zorba-coders/zorba/feature-reference_module/+merge/138345
Your team Zorba Coders is subscribed to branch lp:zorba.


References