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.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to     : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp

Reply via email to