Review: Needs Fixing 1. OMG code duplication: the entire contents of http-client.xq.src is copied (and modified?) from the original http-client module. That's nearly 3000 lines of some of the ugliest and most error-filled code we've got. No way do we want to maintain two copies of it. Also, they both call curl_global_init() and curl_global_cleanup(), which means that they will stomp on each other if anyone attempts to use both. This code must be consolidated somehow, perhaps into a common library that both modules use. Or, we need to simply eliminate the original http-client library. Perhaps we could replace the old http-client module with a pure XQuery wrapper module that replicates the old API, much like V2 of the EXPath http-client module does?
2. Need to consolidate the CMake code which searches for curl, etc. It was copied from modules/com/zorba-xquery/www/modules/CMakeLists.txt, and it still exists in that location. That means the search for CURL is done twice, and the cached variable ZORBA_HAVE_CURL is defined twice, and the Windows .pem files are copied twice... I would suggest that it should remain where it is in modules/CMakeList.txt, and be deleted from the other location, and testing done to ensure it still works right. (This issue would also be resolved by eliminating the original http-client module or replacing it with a wrapper module.) 3. The module error codes need to be changed to match the coding guidelines: http://my.zorba.io/dokuwiki/doku.php?id=coding-guidelines#error_codes -- https://code.launchpad.net/~zorba-coders/zorba/json-http-module/+merge/169579 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : [email protected] Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
