zorba-coders team mailing list archive

[Chris Hillery <chillery+launchpad@xxxxxxxxx>]

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.