zorba-coders team mailing list archive

[Matthias Brantner <matthias.brantner@xxxxxxxxxx>]

Review: Approve

I approve conditionally. There are still some things that require fixing:

1. The following from the diff doesn't make sense.
547	+    zstring lNodeName = child->getNodeName()->getLocalName();
548	+    std::transform(
549	+      lNodeName.begin(), lNodeName.end(),
550	+      lNodeName.begin(), tolower);
551	+

If the options are validated using a schema, there is no need to transform them to
lower case because the schema is case-sensitive. Also, the code should
validate the namespace of the node, not only the local name.

2. Why is x:canonicalize#1 not implemented on x:canonicalize#2 by specifying
the default options in XQuery.

3. x:canonicalize-options-impl needs documentation and should be named
x:canonicalize-impl instead. I think (but I'm not sure) that it doesn't go through
the RQ if the private function doesn't have sufficient parameter documentation.
-- 
https://code.launchpad.net/~zorba-coders/zorba/canonicalize-core-fixed/+merge/142394
Your team Zorba Coders is subscribed to branch lp:zorba.