Code review comment for lp:~zorba-coders/zorba/phpapi

Revision history for this message
William Candillon (wcandillon) wrote :

> - One thing that I don't understand is the relationship with the existing PHP binding.
The low level API enables us to extend/debug the XQueryProcessor API more easily.

> - The first three changes in swig/php/generate_proxy.php.in should be reverted (renaming of libPrefix to prefix)
This is fixed in commit 10597.

> - executeToURI is only capable of writing to files. Also, it currently doesn't stream.
This is fixed in commit 10598.

> - importQueryFromURI only works for file URIs.
This is fixed in commit 10600. There is a test for it in commit 10601.

> - The parse functionality should probably be removed. It merges two functionalities which don't really belong together (retrieving of data and parsing xml). The latter can also be done in XQuery. If this philosophy is to push more functionality into XQuery, the parse functionality should be removed or made symmetric to the existing C++ and XQuery data manager APIs.

This is fixed in commit 10598. Unfortunately there two orthogonal things in commit 10598.

> - getItem doesn't have a comment and is incomplete. Shouldn't it support more/all XQuery types?
It should support any kind of PHP type. Currently objects are not supported. Maybe in the futur we will introduce support for objects.

« Back to merge proposal